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

Adjust for secondary particle energy directly in heating scores #3227

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

pshriwise
Copy link
Contributor

@pshriwise pshriwise commented Dec 16, 2024

Description

This addresses #3207 by adjusting heating scores using the sum of secondary particle energy rather than a placeholder for how many secondaries were produced by the latest reaction. Namely, the attribute Particle::n_bank_secondary_ is replaced with Particle::bank_second_E_ to track how much heating scores should be decremented when secondary particles are generated and will be transported independently.

This also contains a fix for a pathway by which particles were revived from particles in the secondary bank but the original attribute was not reset correctly. This line has been added in Particle::event_revive_from_secondary.

The previous method was more general and I think we should keep it in mind for the future if other/additional information needs to be gathered from the latest secondaries, but updating to this method will avoid the problems we're seeing evidence of when photon transport is enabled with weight windows applied.

Fixes #3207

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@pshriwise
Copy link
Contributor Author

@SteSeg when you have a chance would you mind running this branch through the benchmark you showed in #3207?

@Edgar-21 could you verify that this also solves your problem with secondary revival?

I'll add tests to this PR soon to verify it's fixing the issue.

@pshriwise pshriwise changed the title Adjust for secondary particle energy directly in heating Adjust for secondary particle energy directly in heating scores Dec 16, 2024
@SteSeg
Copy link

SteSeg commented Dec 17, 2024

I'm not getting better results:

image

@pshriwise
Copy link
Contributor Author

@SteSeg hrmm I certainly thought this would help. Are you still getting negative photon heating values? Would you be willing to drop the OpenMC header from those runs in here?

@SteSeg
Copy link

SteSeg commented Dec 17, 2024

Yes, I'm still getting negative contributions from photons but it might be indeed that I'm not correctly checking out this pr:
image

@Edgar-21
Copy link

This seems to have resolved my transport issues, at least it is no longer segfaulting. @SteSeg I agree it looks like you are not using the version of OpenMC in this PR.

@SteSeg
Copy link

SteSeg commented Dec 18, 2024

My bad. It works indeed! This is the situation now:

image

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.

Possible problem tallying photon heating with weight windows enabled
3 participants