Skip to content

Conversation

@jsaffer
Copy link
Collaborator

@jsaffer jsaffer commented May 23, 2025

The original GST paper (http://doi.org/10.1007/s11467-013-0319-7) gives a parameterization of the CR flux in several populations based on p, He, C, O and Fe and heavier mass groups (Table 3). I see that this doesn't match the 5-component model that is used in some working groups (that's why I didn't touch the 5-component model) but for the 4-component version it would be appropriate to use the correct Z values and combine C and O. Also, it is necessary to include the heavier components with Z>50 into the iron group. According to the author (Serap), the 4-population version is to be preferred, that's why I swtiched to it.
I am aware that this creates a difference between GlobalFitGST and GlobalFitGST_IT but the way it has been defined so far is simply incorrect.

jsaffer and others added 3 commits May 21, 2025 17:25
The original GST paper (http://doi.org/10.1007/s11467-013-0319-7) gives a parameterization of the CR flux in several populations based on p, He, C, O and Fe mass groups (Table 3). I see that this doesn't match the 5-component model that is used in some working groups (that's why I didn't touch the 5-component model) but for the 4-component version it would be appropriate to use the correct Z values and combine C and O.
Updating the 4-component GST model to the 4-population version, having C and O combined with the correct Z and Fe combined with the super heavy component
@jsaffer jsaffer requested a review from kjmeagher May 23, 2025 20:23
@jsaffer jsaffer added the invalid This doesn't seem right label May 23, 2025
@github-actions
Copy link

Test Results

    5 files  ±0      5 suites  ±0   1m 58s ⏱️ -1s
  638 tests ±0    412 ✅  - 1    225 💤 ±0  1 ❌ +1 
3 190 runs  ±0  2 059 ✅  - 5  1 126 💤 ±0  5 ❌ +5 

For more details on these failures, see this check.

Results for commit fbbb347. ± Comparison against base commit b79c131.

@kjmeagher
Copy link
Member

I am hesitant to change the value of models that already exist and would prefer to create a new model with a new name and mark the old model as deprecated. If the 5-comp version is similarly incorrect a correct version should also be created and the old version marked as depreciated. Since he was the original author of the 5comp model i have asked @The-Ludwig to coordinate on the correct solution.

Also not the pre-commit is failing because there was a file Level3_IC86.2012_SIBYLL2.1_p_12360_E6.0.i3.bz2 that was erroneously added to the repo. It has been deleted on main, just delete it in this branch as well. The tests are failing because there is regression test using pytest-regression. You need to run pytest --regen-all to update the regression files

@The-Ludwig
Copy link
Contributor

Since he was the original author of the 5comp model i have asked @The-Ludwig to coordinate on the correct solution.

I am super interested in making this clearer and easier to use correctly, but I implented the GSF 5-component version, not the GST 5-component version. Although I would like to take credit for it, the 5-component GST model was in this repository from the initial commit and is thus likely carried over from icetray and predates my time in IceCube.

@kjmeagher
Copy link
Member

Yes it appears i read the blame log incorrectly the original commit is from 2015 https://code.icecube.wisc.edu/projects/icecube/changeset/139164/IceCube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants