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

F90_NONE Removal (MGMC tallying optimization) #2785

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

jtramm
Copy link
Contributor

@jtramm jtramm commented Nov 22, 2023

Description

This PR fixes a small issue in multigroup mode when tallying with an energy filter. The code in question is:

void EnergyFilter::get_all_bins(
  const Particle& p, TallyEstimator estimator, FilterMatch& match) const
{
  if (p.g() != F90_NONE && matches_transport_groups_) {
    if (estimator == TallyEstimator::TRACKLENGTH) {
      match.bins_.push_back(data::mg.num_energy_groups_ - p.g() - 1);
    } else {
      match.bins_.push_back(data::mg.num_energy_groups_ - p.g_last() - 1);
    }
    match.weights_.push_back(1.0);

  } else {
    // Get the pre-collision energy of the particle.
    auto E = p.E_last();

    // Bin the energy.
    if (E >= bins_.front() && E <= bins_.back()) {
      auto bin = lower_bound_index(bins_.begin(), bins_.end(), E);
      match.bins_.push_back(bin);
      match.weights_.push_back(1.0);
    }
  }
}

If we are in multigroup mode and the bins match the XS group structure, then we should be taking the first branch. However, as the groups use 0 based indexing (and F90_NONE is defined as 0) if the particle is at group 0 then it doesn't follow the intended path. Instead, it takes the more expensive continuous energy branch. Thankfully in MG mode the energy gets set to the average of the bin when the particle begins life and when it changes energy, so the correct bin is found, but it would be faster to always take the first path if possible.

The fix is easy -- we can just change F90_NONE to C_NONE (defined as -1), and ensure the particle gets initialized with this value, so that the first branch is always taken in MG mode when the bins match the group structure. This has the added bonus of checking off a "TODO" item from the code comments. As this was the only place that F90_NONE was still being used, we were able to delete it.

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)

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 for digging into this @jtramm!

@paulromano paulromano merged commit 1d4cd9b into openmc-dev:develop Nov 28, 2023
18 checks passed
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
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