Skip to content

Fix coupled external source rate and transfer rate with destination material#3959

Merged
paulromano merged 16 commits into
developfrom
fix_coupled_tr_with_esr
Jul 25, 2026
Merged

Fix coupled external source rate and transfer rate with destination material#3959
paulromano merged 16 commits into
developfrom
fix_coupled_tr_with_esr

Conversation

@church89

@church89 church89 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

This PR enables using external source rates together with transfer rates that specify a destination_material. Previously, the Integrator raised an error if both were set.
Now the following steps are performed:

  1. Augment diagonal matrices first by adding the external source column and auxiliary row of zeros
  2. Augment off-diagonal transfer matrices by adding extra row/column of zeros when destination or source material has an external source, matching the dimension of augmented diagonal matrices.
  3. Assemble the block matrix and solve
  4. Reformat nuclide vectors to match the original size.

The implementation comes from a previous patch of Valerio Mascolino (https://github.com/vmascolino-anl).

Other

Documented missing external source rates in methods/depletion.rst and usersguide/depletion.rst

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)

@church89
church89 requested a review from paulromano as a code owner June 4, 2026 13:34

@paulromano paulromano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix! I did a little bit of cleanup but also made two notable changes:

  • On the branch where both transfer rates and external rates were being used, the call to reformat_nuclide_vectors was done at a point where n_result contained every burnable material (across MPI ranks), but reformat_nuclide_vectors itself only operates on the local materials. I moved the call further down after n_result is distributed back to individual MPI ranks.
  • The clamping of nuclide vectors was redundant in light of #3879 being merged, so I removed it.

@paulromano

Copy link
Copy Markdown
Contributor

@church89 Also for future reference, per our developer's guide, PRs should be submitted from a fork (not a branch pushed to the openmc-dev organization)

@paulromano
paulromano enabled auto-merge (squash) July 24, 2026 15:33
@paulromano
paulromano merged commit ce78bdc into develop Jul 25, 2026
34 of 45 checks passed
@church89

Copy link
Copy Markdown
Contributor Author

Thanks @paulromano for the fixes/improvements and apologies for the oversight.

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