Skip to content

Bundle each IFP data stream with the flag and banks that govern it - #4097

Open
GuySten wants to merge 1 commit into
openmc-dev:developfrom
GuySten:ifp-cleanup-2
Open

Bundle each IFP data stream with the flag and banks that govern it#4097
GuySten wants to merge 1 commit into
openmc-dev:developfrom
GuySten:ifp-cleanup-2

Conversation

@GuySten

@GuySten GuySten commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Bundle each IFP data stream with the flag and banks that govern it

Pure refactor. No behaviour change, no results change, no regolds. Follows on
from the enum removal, and is the second of the pieces split out of #3728.

Problem

IFP tracks two quantities with identical bookkeeping: the delayed group number
of the ancestor neutron (int) and its lifetime (double). Each has a source
bank, a fission bank, and a flag saying whether it is being maintained -- six
globals and two flags, all of which have to stay consistent with each other.

Because they are separate, every operation is written twice:

void resize_ifp_data(vector<T>& delayed_groups, vector<U>& lifetimes, int64_t n)
{
  if (settings::ifp_delayed_on) {
    delayed_groups.resize(n);
  }
  if (settings::ifp_lifetime_on) {
    lifetimes.resize(n);
  }
}

Every function in ifp.cpp has that shape, and eigenvalue.cpp calls the MPI
helpers once per stream behind the same pair of flag checks. Nothing enforces
that a bank is only resized when its flag is set, or that the two banks of one
stream are resized together; that is a convention held up by repetition.

Changes

One type holds a stream's flag and both its banks:

template<typename T>
class IFPStream {
public:
  bool enabled() const;
  void enable();
  void reset();

  void resize_banks(int64_t n_source, int64_t n_fission);
  void store(int64_t i_source, int64_t i_fission, const T& value);
  void copy_from_fission(
    int64_t i_fission, int64_t i_temp, vector<vector<T>>& out) const;
  // ...
private:
  bool enabled_ {false};
  vector<vector<T>> source_bank_;
  vector<vector<T>> fission_bank_;
};

namespace simulation {
extern IFPStream<int> ifp_delayed;     //!< Delayed group numbers
extern IFPStream<double> ifp_lifetime; //!< Neutron lifetimes
}

Every operation is a no-op when the stream is off, so callers stop branching.
ifp() becomes:

void ifp(const Particle& p, int64_t idx)
{
  simulation::ifp_delayed.store(p.current_work(), idx, p.delayed_group());
  simulation::ifp_lifetime.store(p.current_work(), idx, p.lifetime());
}

and the three if (delayed) ...; if (lifetime) ...; pairs in eigenvalue.cpp
become unconditional calls. deserialize_ifp_info takes the stream rather than a
bank, so it can check the flag itself.

settings::ifp_delayed_on and settings::ifp_lifetime_on are gone; the flags
live on the streams. ifp_on() moves to ifp.h alongside them.

free_memory_bank() previously cleared four banks by name and left the flags
alone -- the bug the previous PR fixed separately in free_memory_settings().
It is now a single reset_ifp_streams() that clears both banks and both flags,
so the two cannot drift apart again.

Effect

11 files, +219 / -252.

ifp.cpp goes from 117 lines to 53: six of its eight functions become inline
methods and disappear. ifp.h grows by 56, because the class plus its
documentation costs more than the free functions it replaces, so the net line
count is only modestly better. The gain is structural rather than in the
diffstat: six globals and two flags become two objects, and a flag can no longer
disagree with the banks it governs.

Testing

No results change, so the existing IFP regression and unit tests cover this
unchanged. The MPI paths are exercised by the regression suite when run with
more than one process.

Notes

This makes the remaining piece of #3728 -- delaying activation of IFP tallies --
considerably smaller. That change becomes a matter of calling enable() on the
two streams at the right batch, rather than threading an activation condition
through bank.cpp, eigenvalue.cpp, tally_scoring.cpp and physics.cpp.

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 marked this pull request as ready for review September 1, 2026 19:17
@JoffreyDorville JoffreyDorville self-assigned this Sep 1, 2026
Reset ifp_n_generation in finalize.cpp

Avoid reference into empty vector

Fix formatting

Fix again

ran clang

wip

update

update
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.

2 participants