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

Gaussian Processes Tool #739

Merged
merged 52 commits into from Sep 29, 2023

Conversation

Gaurav17Joshi
Copy link
Contributor

@Gaurav17Joshi Gaurav17Joshi commented Jun 12, 2023

Gaussian Processes Feature for QPO detection

Addressing #660 . This is an implementation for the GP tool (mainly for detection and parameter analysis for QPOs) . This addition makes use of tinygp library for Gaussian Processes implementation, and jaxns for nested sampling.
The new feature is in the stingray.modeling.gpmodelling.py file.
It has three main features:-

  1. get_prior: This function makes the prior function for a specified prior dictionary.
  2. get_likelihood: This function makes the log_likelihood function for the given Kernel, Mean model and lightcurve.
  3. GPResult class: The class which takes a Lightcurve, and performs Nested Sampling for a given prior and likelihood.

Work Remaining To be done:-

  • Add Object Oriented plotting to GPResult.
  • Potentially change the name of file to GP (from GP modelling)
  • Add the Demonstration notebook to Stingray Notebooks.
  • Improve the documentation

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Merging #739 (e9ef4df) into main (5d633c1) will increase coverage by 0.16%.
Report is 76 commits behind head on main.
The diff coverage is 98.21%.

@@            Coverage Diff             @@
##             main     #739      +/-   ##
==========================================
+ Coverage   97.13%   97.29%   +0.16%     
==========================================
  Files          42       43       +1     
  Lines        7929     7918      -11     
==========================================
+ Hits         7702     7704       +2     
+ Misses        227      214      -13     
Files Coverage Δ
stingray/__init__.py 100.00% <100.00%> (ø)
stingray/crossspectrum.py 99.00% <100.00%> (+0.78%) ⬆️
stingray/events.py 99.53% <100.00%> (-0.47%) ⬇️
stingray/fourier.py 99.79% <100.00%> (+0.05%) ⬆️
stingray/lightcurve.py 97.89% <100.00%> (+<0.01%) ⬆️
stingray/lombscargle.py 100.00% <100.00%> (ø)
stingray/powerspectrum.py 99.67% <100.00%> (+0.22%) ⬆️
stingray/pulse/overlapandsave/test_ols.py 100.00% <100.00%> (ø)
stingray/utils.py 98.85% <ø> (+0.35%) ⬆️
stingray/modeling/gpmodeling.py 95.78% <95.78%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Gaurav17Joshi Gaurav17Joshi force-pushed the features/GPtool branch 2 times, most recently from c0019f9 to 7412b3d Compare June 26, 2023 13:08
@Gaurav17Joshi Gaurav17Joshi marked this pull request as ready for review July 26, 2023 16:30
Copy link
Member

@matteobachetti matteobachetti left a comment

Choose a reason for hiding this comment

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

Note that jax will be imported also when you run the test suite and pytest goes through all the modules, so the same try... except ImportError clause you use in the main code should be used here.

@Gaurav17Joshi Gaurav17Joshi changed the title Gaussian Processes Tool [WiP] Gaussian Processes Tool Aug 24, 2023
Copy link
Member

@dhuppenkothen dhuppenkothen left a comment

Choose a reason for hiding this comment

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

Overall, this is looking great! A bunch of comments (some of them possible old and outdates, I have started adding comments before, but I can't see them anymore, so let me know if something pops up that looks weird or that you don't understand), but mostly quite minor. Great work!

stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
stingray/modeling/gpmodeling.py Show resolved Hide resolved
stingray/modeling/gpmodeling.py Show resolved Hide resolved
stingray/modeling/gpmodeling.py Show resolved Hide resolved
stingray/modeling/gpmodeling.py Show resolved Hide resolved
stingray/modeling/gpmodeling.py Outdated Show resolved Hide resolved
@pep8speaks
Copy link

Hello @Gaurav17Joshi! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 40:50: W291 trailing whitespace
Line 622:101: E501 line too long (109 > 100 characters)

Line 25:1: E402 module level import not at top of file
Line 26:1: E402 module level import not at top of file
Line 27:1: E402 module level import not at top of file

@dhuppenkothen
Copy link
Member

The black issue is my fault, I will fix this in the next PR. Other than that, all the tests are passing, all of Matteo's and my nitpicky comments have been addressed, and I think this is ready for merging.

Thank you, @Gaurav17Joshi for your fantastic work! It was a big effort, but I'm so excited that this method is now in stingray!

@dhuppenkothen dhuppenkothen added this pull request to the merge queue Sep 29, 2023
Merged via the queue into StingraySoftware:main with commit 34e6a94 Sep 29, 2023
13 of 15 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.

None yet

4 participants