Skip to content

Conversation

@OlivaresDarian
Copy link

@OlivaresDarian OlivaresDarian commented Nov 26, 2025

Description

This modification allows the Iterated Fission Probability (IFP) method implemented by @JoffreyDorville to break down the effective delayed neutron fraction $\beta_{eff}$ tally by isotope. This extension of the IFP method enables deeper analyses in reactor physics.

In practice, the IFP tally stores certain ancestor information to compute adjoint-weighted tallies. This adaptation follows the same principle but also records the isotope that generated each delayed neutron. It slightly increases both memory use and computation time.

To use it, you simply need to define a list of nuclides in $\texttt{beta-num-tally.nuclides = [...]}$.

Fixes #3605

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)

@GuySten
Copy link
Contributor

GuySten commented Nov 27, 2025

You should run clang format (version 15) on cpp source files for CI to successfully run.

@OlivaresDarian
Copy link
Author

You should run clang format (version 15) on cpp source files for CI to successfully run.

Thanks @GuySten for your help

@GuySten
Copy link
Contributor

GuySten commented Nov 27, 2025

I think you accidentally removed src/output.cpp and its header.
You should watch closely the "files changed" tab of this PR.
A change in 47 files seems too much for a feature of this scope.

@OlivaresDarian
Copy link
Author

I think you accidentally removed src/output.cpp and its header. You should watch closely the "files changed" tab of this PR. A change in 47 files seems too much for a feature of this scope.

You're right. I already restored the output.* files. From the 47 changes, more than 27 are related to tests modifications because we added a new variable to the Particle object. The current version passes all tests in local.

@GuySten
Copy link
Contributor

GuySten commented Nov 28, 2025

But you also added new files (I think by mistake).
Edit:
You can find them by the green plus in the files tab of this PR.

OlivaresDarian and others added 2 commits December 9, 2025 16:46
Co-authored-by: GuySten <[email protected]>
Co-authored-by: GuySten <[email protected]>
Copy link
Contributor

@GuySten GuySten left a comment

Choose a reason for hiding this comment

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

Nice work, @OlivaresDarian!

@GuySten GuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Dec 9, 2025
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.

Putting a placeholder review on this as I'd like to take a look before we merge

@GuySten
Copy link
Contributor

GuySten commented Dec 9, 2025

While trying to fix the current failing tests, I found a bug.
I will update this PR after #3676 is merged.

@JoffreyDorville
Copy link
Contributor

Thanks @OlivaresDarian and @GuySten for this new feature! I would also like to take a look at it before we merge.

@GuySten
Copy link
Contributor

GuySten commented Dec 9, 2025

I think a nice follow up feature will be to sample more frequently delayed neutrons with lower weight.
This should help the convergence of the kinetic parameters.

@JoffreyDorville, @OlivaresDarian, what do you think?

Copy link
Author

@OlivaresDarian OlivaresDarian left a comment

Choose a reason for hiding this comment

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

Hi @GuySten, in commit fcd88d9 to ensure correct results we need to keep the lines that were removed here. Inside the if block, the upper bracket uses delayed_groups[0], while after the else block it uses delayed_groups[1]. This is because ancestor_event_nuclide[0] stores the isotope from which the neutron in delayed_groups[1] was born (the next generation).

Regarding your proposal, I need to investigate a bit more, but it should be possible.

@GuySten
Copy link
Contributor

GuySten commented Dec 10, 2025

My bad, I thought that was a typo.
I fixed it in the last commit.

Copy link
Contributor

@GuySten GuySten left a comment

Choose a reason for hiding this comment

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

I added a suggestion to prevent code repetition.

OlivaresDarian and others added 6 commits December 11, 2025 12:28
…age to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@GuySten GuySten removed the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Dec 13, 2025
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.

Nuclide filter for IFP method

4 participants