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

Adding neural network inference to NMMA #370

Merged
merged 112 commits into from
Jul 18, 2024

Conversation

malinadesai
Copy link
Contributor

A new function has been added to /nmma/em/ananlysis.py called nnanalysis that can accept either a light curve or an injection file and will produce a corner plot displaying the inference results. Helper functions and model weights are added to nmma/mlmodel. Running this will require the following two commands to be added when creating the nmma environment: conda install pytorch torchvision -c pytorch
pip install nflows

changed main to have a neural net option
started adding nn analysis function
indentation error
added missing colon
added print statements
fixed indentation
commented out code for debugging
removed all added code
adding print statements
changed logic in def main
defined num points
added missing colon
changed main function
uncommented filter requirements
added print statement
added dt logic
set up logger and outdir
changed tmin tmax logic
added tmin, tmax, num points, current points
adding injection file option
changed print statement
light curve generation
@tsunhopang
Copy link
Collaborator

@malinadesai could you update the requirement to include the new packages required?

@sahiljhawar
Copy link
Member

sahiljhawar commented Jul 16, 2024

@malinadesai can you also add a test for neuralnet. See here

args.interpolation_type = "sklearn_gp"

Also, could you add the conda command (and any pitfalls that user might encounter) in the documentation (Installation page) and also a brief intro on how to use the neuralnet. I suggest to open an new PR for the documentation.

@malinadesai malinadesai marked this pull request as draft July 17, 2024 19:49
Copy link
Member

@sahiljhawar sahiljhawar left a comment

Choose a reason for hiding this comment

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

Can you also add the input files required, I can try adding the test.

added instructions for setting up env
added test function for nn_analysis
@malinadesai
Copy link
Contributor Author

@tsunhopang @sahiljhawar I added an ml_requirements.txt file that contains torch and nflows, and i was able to successfully create a new environment when running pip install -r ml_requirements.txt. I also added lfi_analysis.md to the doc directory with example use and addressing the limitations. I also added a new test function to nmma/tests/analysis.py called def test_nn_analysis(args). I'm not sure if that function is the appropriate way to add a test, so it could definitely benefit from a review.

@sahiljhawar
Copy link
Member

hehe, thanks for the test

@sahiljhawar
Copy link
Member

@mcoughlin @tsunhopang is there a need of refactoring since nnanalysis have a lot of similar code to analysis?

@mcoughlin
Copy link
Member

@sahiljhawar @tsunhopang I suggest prioritizing getting this merged, and work on a refactor in a second PR.

@sahiljhawar
Copy link
Member

Tests are passing, LGTM

@mcoughlin mcoughlin marked this pull request as ready for review July 18, 2024 15:43
@mcoughlin
Copy link
Member

@sahiljhawar feel free to give an approving review and merge!

@sahiljhawar sahiljhawar merged commit 9e88cff into nuclear-multimessenger-astronomy:main Jul 18, 2024
2 of 4 checks passed
@sahiljhawar
Copy link
Member

@malinadesai you should reset your main branch with NMMA main branch. As @mcoughlin once said, "Please use a branch for next time."

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.

4 participants