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

Keep interactions even if outside NEST validity #241

Merged
merged 6 commits into from
Jun 27, 2024

Conversation

cfuselli
Copy link
Member

@cfuselli cfuselli commented Jun 12, 2024

Instead of removing the interaction, keep it and just give the warning.

I find the "Remove interaction" much more dangerous than using a yield model that is not completely validated by data from NEST. The function can return values also for energies much above the validity range, and seems to give reasonable results.

Instead, removing interactions can result into tricky results if the analyst is not fully aware of the possibility.

Here attached the model validity and the results of nc.GetYields() for energy ranges outside the validity ( 3MeV for gamma and beta, 200keV for NR).

test_fuse_yields_nest_nr_yields test_fuse_yields_nest_gamma_yields test_fuse_yields_nest_beta_yields

Just to make it extra clear:

  • the smooth solid lines are the output of nestpy.PhotonYield() and nestpy.ElectronYield(), they give NaN outside the validity range.
  • the lines that oscillate more are the output of nc.GetYields().getQuanta() - that is what is used in the plugin, and gives values also outside the validity range.

The functions that define the yields model come from here in NEST.

Instead of removing the interaction, keep it and just give the warning
@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9484437464

Details

  • 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
  • 28 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.08%) to 78.396%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fuse/plugins/micro_physics/yields.py 1 2 50.0%
Files with Coverage Reduction New Missed Lines %
fuse/plugins/micro_physics/yields.py 28 76.43%
Totals Coverage Status
Change from base Build 9451990213: 0.08%
Covered Lines: 2326
Relevant Lines: 2967

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9484444416

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 78.396%

Totals Coverage Status
Change from base Build 9451990213: 0.08%
Covered Lines: 2326
Relevant Lines: 2967

💛 - Coveralls

Copy link
Collaborator

@HenningSE HenningSE left a comment

Choose a reason for hiding this comment

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

Thanks, @cfuselli I agree: Just removing these energy deposits is dangerous and will harm us more than having a questionable yield model. I would propose merging this PR with the other high-energy-focused PRs.

@HenningSE
Copy link
Collaborator

Ohh and one more thing: Can you bump the plugin version?

Copy link
Collaborator

@ramirezdiego ramirezdiego left a comment

Choose a reason for hiding this comment

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

Forgot to approve this after discussing with Carlo 😆 Thanks, Henning!

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9647500637

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 78.449%

Totals Coverage Status
Change from base Build 9647484856: 0.08%
Covered Lines: 2337
Relevant Lines: 2979

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9694111370

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 80.532%

Totals Coverage Status
Change from base Build 9694094094: -0.02%
Covered Lines: 2602
Relevant Lines: 3231

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9694124065

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 80.532%

Totals Coverage Status
Change from base Build 9694094094: -0.02%
Covered Lines: 2602
Relevant Lines: 3231

💛 - Coveralls

@ramirezdiego ramirezdiego merged commit de0d8bf into main Jun 27, 2024
4 checks passed
@ramirezdiego ramirezdiego deleted the keep-nest-oustide-validity branch June 27, 2024 09:42
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