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

Caching and test for GP #224

Conversation

sahiljhawar
Copy link
Member

This PR implements the use of caching to save the SVD models as cache and use them for all the future runs without the need to download models everytime. Fetching models from Zenodo is really slow, so for now the svdmodel.tar is hosted on Potsdam server. Following this, the analysis test has been updated to perform test on Bu2019lm since it makes use of the model. Also, note that if the tests are being used as is, do not use or create (from a code) any local folder by the name svdmodels as this can cause problems.

Note for admins: I would advise you to clear all the caches of this repo in order to have a fresh start.

Copy link
Member

@mcoughlin mcoughlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sahiljhawar
Copy link
Member Author

sahiljhawar commented Sep 8, 2023

Don't merge, injection test is failing randomly.

@sahiljhawar
Copy link
Member Author

Removed GRB model from injections.py test. Also, need to check the availability of caches from main branch to feature branch.

@sahiljhawar sahiljhawar closed this Sep 8, 2023
@sahiljhawar sahiljhawar reopened this Sep 8, 2023
@sahiljhawar
Copy link
Member Author

sahiljhawar commented Sep 8, 2023

Cache present on the main branch cannot be accessed by the feature branches. This means, whenever PR comes, new caches are added to the repo alongside the cache of the main branch. Commit 5b374d4 adds a post PR merge (and closed) cache deletion workflow.

@sahiljhawar
Copy link
Member Author

@mcoughlin This can be merged now. Tests are passing. Cache workflow will be in action after the merge. In another PR I will pin parameters for GRB.

Copy link
Member

@mcoughlin mcoughlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mcoughlin mcoughlin merged commit d570118 into nuclear-multimessenger-astronomy:main Sep 8, 2023
5 checks passed
@sahiljhawar sahiljhawar deleted the updated_test_CICD_pipeline branch September 8, 2023 18:37
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

Successfully merging this pull request may close these issues.

2 participants