-
Notifications
You must be signed in to change notification settings - Fork 58
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
Restrict scikit-learn version to <1.2 #231
Restrict scikit-learn version to <1.2 #231
Conversation
does this have any impact on the version of numpy that is installed? I know I've encountered a couple of warnings when setting up a new nmma environment regarding the compatibility of scikit-learn and numpy versions |
@bfhealy @tylerbarna I will check with Ari about getting these updated, but for now, this seems good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@bfhealy do you know if there's any compatibility issues with numpy/pandas? |
@tylerbarna So far I haven't encountered any. I set up a new environment today and didn't get any related warnings/errors. |
@bfhealy interesting, what python version did you set up your environment with, and did you use pip or conda for installing parallel-bilby? |
I'm getting a new, fun error relating to pandas when trying to generate some lightcurves from an injection of like 100. I was able to resolve the version warning by pinning numpy to 1.22.4 (at least on python 3.10), but I hit the following error:
|
@tylerbarna I used python 3.10 and conda installed parallel-bilby (version 2.0.2). My numpy version is 1.24.3, and pandas is 2.1.0. Could you please share the commands you ran so I can give them a try? |
@bfhealy here's the repo, should work by just running the script with the nmma environment active |
@bfhealy were you able to try running it? It occurs to me it depends on there being another repo for the priors, which is https://github.com/tylerbarna/dsmma_kn_23/tree/main/priors I just pushed a commit that should include a copy of the priors inside the nmma-model-recovery repo so there isn't that dependency |
@tylerbarna Thanks for adding the priors. While running your script I encountered the same |
@bfhealy would you mind pulling one more time and running a basic analysis on one of the generated lightcurves I pushed, something like
I've been encountering an issue with my ev.dat being empty and haven't been able to figure out if it's an issue with the environment or something in the way I'm generating the lightcurves |
@tylerbarna I ran that command and sampling completed successfully. I got an ev.dat file that's 2MB in size. Perhaps a fresh environment installation would help? I do get several warnings about a change in prior name: |
@bfhealy very odd, I've encountered this same issue on two different systems now, both MSI and my local PC (WSL).
|
@tylerbarna Hmm, since the issue is with the sampling I wonder if it has to do with pymultinest. There was a recent new release of version 2.12 on pypi. Maybe it's worth upgrading that if you haven't already? |
@bfhealy just checked, looks like pymultinest is already on version 2.12 |
12:06 bilby INFO : Using temporary file /tmp/tmpq58ivb8q MultiNest v3.10 no. of live points = 2048 Starting MultiNest This resuming from previous job seems pretty weird. |
This PR restricts the scikit-learn version in
requirements.txt
to <1.2 (in addition to >=1.0.2). Assuming the tests pass with the different package version, this resolves #173. The.joblib
files loaded when setting--ztf-uncertainties
and--ztf-sampling
were likely generated using an older version of scikit-learn, raising unusual errors when attempting to load them with newer versions of the package. A longer-term solution will be to regenerate those files with newer code so we don't need to constrain the package version.