force both materials to decay on absorption

[gonuke]

Summary of Changes

Force both materials involved in an absorption to decay before absorption.

Design Notes

Previous behavior would result in dubious decay times for materials following absorption

Importantly, this PR introduces a new constraint on material decay:

• tracked materials can only decay forward and up to the current context time

Check the box if your change does not break any of the following:

• API for Cyclus modules
• Input files
• Output tables for post processing tools

Related CEPs and Issues

Fixes #1841

Associated Developers

@dean-krueger

Testing and Validation

• I have read the Contributing to Cyclus guide
• I have compiled and run the code locally
• I have added or updated relevant tests
• I have added documentation for new or changed features
• This code follows the style guide
• I have updated the changelog

Reviewers, please refer to the Cyclus Guide for Reviewers.

[gonuke]

This might just need to update the expected responses for tests to allow it to pass (?)

[dean-krueger]

Seems like the two failing tests are these:

MaterialTest.AbsorbPrevDecay and ResourceTest.MaterialUnitValue

MaterialTest.AbsorbPrevDecay has a note in it that reads:

// The behavior this test checks for is subject to change and is not part of
// the Cyclus API.  It is testing "undefined" implementation detail behavior
// as coded.  We may decide to change the behavior in the future breaking
// this test; the test will need to be modified accordingly.
//
// This test checks to see that, when materials are absorbed together, the
// previous decay time for the larger quantity material is used as the value
// for the new, combined material.

Which indicates that we should be good to change this test.

ResourceTest.MaterialUnitValue is a test I added when doing the material value stuff for Cyclus Economics work, and I'd be willing to bet that I just had Cursor make it work with the old decay behavior. I'll look into changing this as well. Expect a PR into this PR soon.

[dean-krueger]

The UnitValue test seems to be failing because it looks like you accidentally took out the unit-value averaging lines of the absorb function. Adding those back now and then it should (hopefully) work!

[dean-krueger]

PR submitted to @gonuke's branch which, if accepted, should come here and hopefully tests pass after that.

[dean-krueger]

@gonuke has brought up some good points about more careful testing that needs to happen now that this change has been implemented. They are:

Regarding the context:

• In the changes that I proposed in my PR force both materials to decay on absorption #1918, there is the possibility for absorb to happen for materials that are not governed by a context.

• the previous behavior of Material::Absorb() didn't care about a context because it only picked the decay time of one of the two materials.

• I added a check for the context so that we could advance the decay time to the current context time, rather than only the times in the materials.

• I can't remember all the ways that we might arrive at materials being outside of a context, but most of the tests for decay throughout the material tests do not assume a context.

• if we only want to perform Absorb() on materials in a context, then Material::Absorb() needs to be updated to check that it's true and fail/warn otherwise.

• if we want to allow Absorb() for materials not in a context, then we should include tests for that.

Regarding backward decay:

• I don't think it's every been considered a normal behavior

• if we always want to force decay to move forward we need to not just use the context time, but take the max of the previous decay times and the context time to determine the new time

• we can also fail/warn if the previous decay times are ever beyond the current context time

• if we leave it as is, it's possible that we cause decay to go backwards

• whatever we choose, we need to test it appropriately

We (likely me) should look into adding these tests to improve our coverage.

[dean-krueger]

Checks have been added to my PR into @gonuke's PR addressing the above. Hopefully those go through, and this can move forward, but obviously if I did something funky let me know and I can address it.

[coveralls]

Coverage Report for CI Build 28475723402

Warning

No base build found for commit c972f49 on v1.6.1-pre.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 36.831%

Details

• Patch coverage: 91 of 91 lines across 4 files are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	164129
Covered Lines:	60451
Line Coverage:	36.83%
Coverage Strength:	14753.86 hits per line

---

💛 - Coveralls

[github-actions]

Downstream Build Status Report - 0ed2078 - 2026-06-30 11:50:13 -0500

Build FROM cyclus_22.04_apt/cyclus

• Cycamore: Success ✅
• Cymetric: Failure ❌

Build FROM cyclus_22.04_apt/cyclus --parallel

• Cycamore: Success ✅
• Cymetric: Failure ❌

Build FROM cyclus_22.04_conda/cyclus

• Cycamore: Failure ❌
• Cymetric: Skipped due to upstream failure ⚠️

Build FROM cyclus_22.04_conda/cyclus --parallel

• Cycamore: Failure ❌
• Cymetric: Skipped due to upstream failure ⚠️

Build FROM cyclus_24.04_apt/cyclus

• Cycamore: Success ✅
• Cymetric: Failure ❌

Build FROM cyclus_24.04_apt/cyclus --parallel

• Cycamore: Success ✅
• Cymetric: Failure ❌

Build FROM cyclus_24.04_conda/cyclus

• Cycamore: Failure ❌
• Cymetric: Skipped due to upstream failure ⚠️

Build FROM cyclus_24.04_conda/cyclus --parallel

• Cycamore: Failure ❌
• Cymetric: Skipped due to upstream failure ⚠️

[dean-krueger]

Now that gone/cyclus/#2 has been merged, I THINK the main thing on this is fixing the Cycamore tests. Let me know if you see other stuff that needs addressing.

Another thought I was having while making the tests work yesterday was that when we decay outside of a context we give it a number of months to decay (not time steps, since it's not in a context). It MIGHT make more sense to do it in seconds and then make people who want months use N * cyclusMonth as the argument so that you can do any timestep.

[dean-krueger]

Okay, the giant wave of failing Cycamore tests I saw previously seems to have been related to the != !mat->HasContext thing, but there was still one Cycamore test which failed. I've made a new PR into this PR that resolves it. Hopefully that ends up being enough to push this over the finish line! Give that a look whenever you get a chance @gonuke

[gonuke]

Replaced by #1966