Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We ran into two problems when using ms_pred as an installed package rather than from a clone of the repository.
iceberg_prediction() in iceberg_elucidation.py starts predict_smis.py using the path src/ms_pred/iceberg/predict_smis.py, which only works when running from the root of the repo. From anywhere else the call returns without writing any predictions. The script is now located relative to iceberg_elucidation.py itself.
PyTorch Lightning 2.0 removed pl.utilities.seed.seed_everything, so the scripts that still call it stop with an AttributeError. We've switched the 26 remaining calls to pl.seed_everything, which works in both 1.x and 2.x. The three places that already have a fallback (iceberg/predict_gen.py, marason/predict_gen.py and marason/predict_smis.py) are left as they are.
We tested the path fix on CPU with the MassSpecGym ICEBERG 2.1 checkpoints. Run from outside the repo, iceberg_prediction() writes no preds.hdf5 on main, but does with this change.
#35 fixes the same path problem, but against the older dag_pred layout, so it no longer applies cleanly.