Skip to content

Correctly score pulse height tally when no cell filter is present#3821

Merged
paulromano merged 4 commits intoopenmc-dev:developfrom
GuySten:pht-cell-filter-check
Feb 26, 2026
Merged

Correctly score pulse height tally when no cell filter is present#3821
paulromano merged 4 commits intoopenmc-dev:developfrom
GuySten:pht-cell-filter-check

Conversation

@GuySten
Copy link
Contributor

@GuySten GuySten commented Feb 18, 2026

Description

Currently when no CellFilter is specified, pulse height tally results are always 0.
This PR fix that and now when no cell filter is specified, all the geometry is scored to.

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)

@cfichtlscherer
Copy link
Contributor

Thanks for your work again. I am not sure if we need this. Why would you want to know the pulse-height over the entire geometry when no CellFilter is provided?

@GuySten
Copy link
Contributor Author

GuySten commented Feb 18, 2026

I discovered this unexpected behavior while trying to test a simple example containing one cell.

This can be useful for example when you have a model of one cell that is the detector vacuum boundary conditions and a fixed source.
You should be able to tally the pulse spectrum over the whole geometry (the detector) and you can model the difference between large and small detectors by changing the cell dimensions.

Now you have to specifiy a cell filter with that one cell as a workaround.

@vitmog
Copy link
Contributor

vitmog commented Feb 19, 2026

Each filter must have the default "neutral" form $f(x) \equiv 1$. Otherwise, it would be necessary to define all available filters for each detector.

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Thanks @GuySten. I've made three refinements to improve code clarity and correctness:

  • Eliminated code duplication in the SCORE_PULSE_HEIGHT case by consolidating the two branches into a single loop with a ternary operator for the cell index.
  • Fixed a bug in tally_scoring.cpp where assigning to a mutable reference was inadvertently mutating model::pulse_height_cells, which could corrupt scoring for multiple pulse-height tallies. This now uses a const reference to safely bind either the filter's cells or the global vector.
  • Changed pulse_height_cells from vector<int> to vector<int32_t> to match CellFilter::cells() for consistency.

@paulromano paulromano enabled auto-merge (squash) February 25, 2026 18:49
@paulromano paulromano disabled auto-merge February 25, 2026 18:50
@paulromano paulromano changed the title Pulse height does not score over whole geometry when no cell filter present. Correctly score pulse height tally when no cell filter is present Feb 25, 2026
@paulromano paulromano enabled auto-merge (squash) February 25, 2026 18:50
@paulromano paulromano merged commit 3ba8a9f into openmc-dev:develop Feb 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants