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

Add code to generate slurm script for lightcurve-analysis on HPC #248

Merged

Conversation

bfhealy
Copy link
Collaborator

@bfhealy bfhealy commented Sep 25, 2023

This PR adds new code (tools/analysis_slurm.py) to generate a slurm script (named slurm.sub by default) to run lightcurve-analysis on an HPC resource. The code parses all current arguments in em/analysis.py along with slurm-specific arguments in order to allow maximum customizability.

The idea is that this code could be run one time on an HPC resource before being queued (e.g. via API calls) many times for different models/sources. Thus, a few of the defaults for analysis.py arguments are changed. These include --model, --label, --trigger-time, and --data, which all take environment variables by default ($MODEL, $LABEL, $TT, $DATA). These arguments can be customized upon script submission by exporting the appropriate environment variables in the call to sbatch, e.g:

sbatch --export=MODEL=Bu2019lm,LABEL=test,TT=59361.0,DATA=example_files/candidate_data/ZTF21abdpqpq.dat slurm.sub

Also, the default prior is priors/$MODEL.prior.

@bfhealy bfhealy added this to the NMMA API milestone Sep 25, 2023
@sahiljhawar
Copy link
Member

LGTM.

However instead of redefining args it can be imported from analysis.py (get_parser()), this way this script need not be updated explicitly

Copy link
Collaborator

@Theodlz Theodlz left a comment

Choose a reason for hiding this comment

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

LGTM! Same comment about the parser, importing it would be better.
As long as this allows us to pass the data from SP and retrieve the results, I'm very happy with this :)

@sahiljhawar
Copy link
Member

sahiljhawar commented Sep 26, 2023

LGTM. Is it merge-ready?

@bfhealy
Copy link
Collaborator Author

bfhealy commented Sep 26, 2023

@sahiljhawar Yes, ready to merge!

@sahiljhawar sahiljhawar merged commit 0e395c8 into nuclear-multimessenger-astronomy:main Sep 26, 2023
5 checks passed
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.

3 participants