Pin lalsuite version and fix error in pycbc_plot_singles_vs_params #4997
Conversation
GarethCabournDavies
force-pushed
the
plot_singles_bug
branch
from
03fad85 to
503488e
Compare
|
This is testing the bugfix now |
.github/workflows/basic-tests.yml
Outdated
| @@ -32,7 +32,7 @@ jobs: | |||
| cd lalsuite-extra | |||
| git lfs pull -I "data/lalsimulation/SEOBNRv2ROM_*.dat" | |||
| git lfs pull -I "data/lalsimulation/*ChirpTime*.dat" | |||
| git lfs pull -I "data/lalsimulation/SEOBNRv4ROM_v2.0.hdf5" | |||
| git lfs pull -I "data/lalsimulation/SEOBNRv4ROM_v*.hdf5" | |||
There was a problem hiding this comment.
Curious about this. Looking at the Gitlab repo, that file has not been changed in 8 years (!).
There was a problem hiding this comment.
Yup, looking deeper in the errors, it seems to be wanting v3.0 of that file, and it doesn't exist that I can see
There was a problem hiding this comment.
In fact it could be this change that caused the error
There was a problem hiding this comment.
Wait, is this on a lalsuite release? How did that slip through??
There was a problem hiding this comment.
Blocking the latest lalsuite version is a viable workaround (latest tests), but I know that we generally don't like blocking versions of dependencies.
It is not planned for the required files to be on lalsuite-extra, so we will need to access an alternative.
I think blocking that version for now until the new location is decided, and then we can unblock it once we point to the new location
There was a problem hiding this comment.
Pinning lalsuite to less than this version might be needed temporarily (don't block the release, pin to less than)
GarethCabournDavies
changed the title
This was a testing PR for some error in
pycbc_plot_singles_vs_params, but I identified and fixed an error with lalsuite during the testing, so it is now both fixes.The changes are discussed in the comments (and linked issue), but the summary is:
The matplotlib version testing failed when 3.10 was used, as it did not understand semantic versioning, this has now been added
The lalsuite version mentioned requires a data file which is not public, and so we pin to before that version until the data file is public and we can point to it
Fixes #4993