Skip to content

Do not count zero energy pulse heights#3938

Open
GuySten wants to merge 4 commits into
openmc-dev:developfrom
GuySten:pulse-height-test
Open

Do not count zero energy pulse heights#3938
GuySten wants to merge 4 commits into
openmc-dev:developfrom
GuySten:pulse-height-test

Conversation

@GuySten
Copy link
Copy Markdown
Contributor

@GuySten GuySten commented May 14, 2026

Description

Currently openmc tally zero energy pulse heights.
This PR fix that by skipping zero energy pulse heights.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) 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)

@GuySten GuySten added the Bugs label May 14, 2026
@cfichtlscherer
Copy link
Copy Markdown
Contributor

The change makes sense to me, physically a detector doesn't register a "zero pulse," and it brings the PHT in line with how every other tally in OpenMC behaves (no event -> no score). That said, there is an implicit PHT convention coming from MCNP's F8 tally. MCNP as well keeps zero-energy events and expects the user to partition them out by adding a zero bin and an epsilon bin (~1e-5) on the E card (see Booth, LA-13955). That's the motivation for the current design choice: expose the events, let the user handle them.

So I lean towards keeping it as it is. If we do this design change, some other things to consider:

@GuySten
Copy link
Copy Markdown
Contributor Author

GuySten commented May 17, 2026

In the example you linked there is no epsilon bin.
So if we do not do this change we will have to update the example.

I lean towards not scoring zero pulse events.
I do not understand why someone will want to track zero energy pulse events.

We could also decide to ignore pulses under 10eV so we will not need epsilon bin.

@paulromano, what do you think?

@GuySten GuySten removed the Bugs label May 18, 2026
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.

2 participants