Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using robustica with numpy>=2.0 #6

Closed
handwerkerd opened this issue Sep 3, 2024 · 9 comments
Closed

using robustica with numpy>=2.0 #6

handwerkerd opened this issue Sep 3, 2024 · 9 comments
Assignees

Comments

@handwerkerd
Copy link

I'm part of a group that is adding robustica into https://github.com/ME-ICA/tedana

I just came across an issue that packages that use numpy and were last compiled before the release of numpy v2.0 don't run when numpy>=2.0. That means pip install robustica no longer works if an environment uses a newer version of numpy.

This is the message I am seeing:

>>> from robustica import RobustICA

A module that was compiled using NumPy 1.x cannot be run in
NumPy 2.1.0 as it may crash. To support both 1.x and 2.x
versions of NumPy, modules must be compiled with NumPy 2.0.
Some module may need to rebuild instead e.g. with 'pybind11>=2.12'.

If you are a user of the module, the easiest solution will be to
downgrade to 'numpy<2' or try to upgrade the affected module.
We expect that some modules will need time to support NumPy 2.

I'm assuming, if you release a new version of robustica using an environment with a newer numpy version, it should work with newer and older versions of numpy. This is a bit out of my expertise, but I don't think any code changes are necessary.

I'm not sure this is relevant here, but if you want to see the details of the PR where we're adding robustica, it's here: ME-ICA/tedana#1125 (comment)

Thank you for creating and openly sharing robustica!

@MiqG
Copy link
Collaborator

MiqG commented Sep 4, 2024

Hi @handwerkerd , thanks for noting this and posting the issue. I will look into it asap!

Best,

Miquel

@MiqG MiqG self-assigned this Sep 4, 2024
@handwerkerd
Copy link
Author

handwerkerd commented Sep 6, 2024

Thank you @MiqG! I've been looking into this a bit. I tried to locally install robustica and was getting the same numpy version issue. I then realized the problem might also be that scikit-learn-extra has not release a new version since 2023 and that giving the same A module that was compiled using NumPy 1.x cannot be run error. That means, to use robustica with numpy>2.0 it would either need to remove the scikit-learn-extra dependency or ask that project to release a newly compiled version. I hope this helps.

[Update] It seems like this is a partially known issue: scikit-learn-contrib/scikit-learn-extra#177 and that issue makes it seem like a solution might take them time.

[Update2] Based on the above thread, if I uninstall and then reinstall scikit-learn-extra using

pip install git+https://github.com/TimotheeMathieu/scikit-learn-extra

this error disappears and then I can also successfully import robustica. I think this means, to get a new version of your code up pipy that can use numpy>2.0 you might need to wait for scikit-learn-extra to resolve some other issues in their code and release a new version first.

@MiqG
Copy link
Collaborator

MiqG commented Sep 9, 2024

Oh I see! I just updated the README to inform users about that.

Thanks for digging into the bug @handwerkerd !

@handwerkerd
Copy link
Author

@BahmanTahayori was able to remove scikit-learn-extra and this overall issue is resolved primarily by removing two options in clustering_defaults. He has an updated version of the code here: https://github.com/BahmanTahayori/robustica
Would you consider merging this into your code and removing KMedoids and CommonNNClustering as options, or might that cause other issues for you? It might also be possible to change scikit-learn-extra into an optional module so that robustica can run even if it's not installed.

In general, I'd like to get a sense if you have the bandwidth to fix this in your repo or if, for our project, we should go forward with our own fork that removes scikit-learn-extra

MiqG added a commit that referenced this issue Sep 12, 2024
…des whiten default from whiten=True to whiten=arbitrary-variancefollowing latest scikit-learn updates RobustICA class.
@MiqG
Copy link
Collaborator

MiqG commented Sep 12, 2024

Hi @handwerkerd , this is easily fixable!

In this case, rather than removing it completely, I think making scikit-learn-extra optional is the best way to go as the clustering algorithms they offer perform quite well in our benchmarks (see DOI: https://doi.org/10.1186/s12859-022-05043-9).

I just committed some changes that in my end allow installing and running the package without requiring scikit-learn-extra.

Could you give it a try as well installing the latest version of the repo? If it works on your end as well, I will make the release of a new version and update pypi:

pip install git+https://github.com/CRG-CNAG/robustica.git

My intention is to keep the repo as active and up to date as possible. Of course contributions are very welcome!

@handwerkerd
Copy link
Author

This works. Thank you! I'm also glad to hear you plan to keep this active & up to date.

I will mention two things that might need edits:

  1. I think you still have version 0.1.3 in the code and might want to change the number.
  2. When I install a local version using pip install -e I'm seeing a warning that your type of setup.py will stop working with pip 25.0 (5-6 months away). If you're releasing a new version anyway, it might be a good time to fix that. Here's the message I see when I installed it:

DEPRECATION: Legacy editable install of robustica[all]==0.1.3 from file:///Users/handwerkerd/code/meica/robustica (setup.py develop) is deprecated. pip 25.0 will enforce this behaviour change. A possible replacement is to add a pyproject.toml or enable --use-pep517, and use setuptools >= 64. If the resulting installation is not behaving as expected, try using --config-settings editable_mode=compat. Please consult the setuptools documentation for more information. Discussion can be found at https://github.com/pypa/pip/issues/11457

@MiqG
Copy link
Collaborator

MiqG commented Sep 12, 2024

Great, I think I'll be able to implement this by next week.

@MiqG
Copy link
Collaborator

MiqG commented Sep 16, 2024

Done!

@handwerkerd
Copy link
Author

Thank you for taking care of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants