-
Notifications
You must be signed in to change notification settings - Fork 0
Add Global Spline Fit model #10
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
Conversation
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 669 693 +24
=========================================
+ Hits 669 693 +24
☔ View full report in Codecov by Sentry. |
|
@kjmeagher From my side this is ready for review. As said, the pre-commit is failing, since I do not know how to give the right license to the GSF data table. |
kjmeagher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good aside from the comments above. For the data file license you can ask @ @HDembinski if he can license it under BSD-2-Clause
Co-authored-by: Kevin Meagher <11620178+kjmeagher@users.noreply.github.com>
|
Hi Ludwig, btw IceCube internally should have the GSF model code already. For public code, it is an acceptable compromise to use the public tables, as you do. The tables themselves are not under any license, they are not source code. Perhaps I should nevertheless clarify under what permissions one can use the tables. I hereby declare that the tables are available under CC BY 4.0 https://creativecommons.org/licenses/by/4.0/. Please add a license file to the package and indicate that the tables fall under that license. Scipy is a good example on how to handle code/data licensed under compatible licenses, see e.g. https://github.com/scipy/scipy/blob/main/LICENSES_bundled.txt |
|
@HDembinski Thanks for the reply! I added the license, although I'm not sure how to give details about compatibility. @kjmeagher Pre-commit is now failing because of the large scipy dependency, what can we do about that? |
|
CC BY 4.0 is compatible with BSD 2-clause, in the sense that both give you similar distribution and modification rights. I chose CC BY 4.0 mainly because it guarantees attribution. |
kjmeagher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good. Thanks to @HDembinski for the relicense.
Hi, first off: Sorry again for pushing to main.
I implemented the GlobalSplineFit primary model by Dembinski et. al. Proceeding for the model can be found here and an overview of the model as well as the data can be found here.
This model is, as far as I understand it, the state-of-the-art model right now and is actually fited with real data.
Unfortunately, the model is not yet publicly available, yet, since the developers are still working on the paper. Once the paper is released, they will also release a python package. The best thing to do right now (according to Hans Dembinski) is to download the tabulated data and interpolate between the values.
Of course, this raises questions for this pull request, as it is unclear how to distribute these tabulated values and how the copyright on that will work (that's also why the Pre-Commit tests will fail).
I think shipping the .txt file with the source code is okay for now. Sadly, the website does not seem to have an api, which would permit downloading the table in an install script. The other option I can think of is to upload the table in an release as an asset and downloading that asset in the install script.
To interpolate I use
scipy.interpolate.CubicSpline. Since I did not want to bloat the nice and short dependency list, I added it as an optional dependency and raise an error if scipy is not found.Anyway, please let me know what you think about this and how I can improve this PR.
Context
For our prompt-muon analysis this model will be very interesting to test, since it does seem to produce the highest prompt-muon fluxes compared to other models.