Summary
construct_old_react_source() takes U_state and R_source, but in the 4th-order averaged-input path it also writes centered reaction data into the class member Sburn. That side effect is not visible in the interface and makes the function unsafe to reuse without knowledge of internal scratch-state conventions.
Affected code
Problem
In the sdc_order == 4 && input_is_average branch, the function computes centered reaction rates and then stores them into Sburn:
Sburn[mfi].copy(R_center, obx, 0, obx, 0, NUM_STATE);
The function signature and header comment only advertise output through R_source, not through Sburn. That means the routine has a hidden dependency on mutable object state and a hidden output channel.
Why this matters
- A future caller can easily assume the function only fills
R_source.
- Any code expecting
Sburn to still contain an initial guess or scratch state after the call can be broken implicitly.
- The coupling makes the routine harder to reason about, test, or refactor.
Current risk
The current call sites appear to rely on or at least tolerate this behavior, so this may not be an immediate user-visible bug today. But it is a maintenance hazard and an easy source of future regressions.
Suggested fix
Choose one of these and make it explicit:
- Remove the side effect and return/store the centered reaction source through an explicit output argument.
- Rename the function or document clearly that it also populates
Sburn.
- Split the responsibilities into two helpers:
- one that computes
R_source
- one that optionally caches centered reaction data for plotting or later use
Possible acceptance criteria
- The public declaration/comment reflects every mutated output.
- No helper mutates
Sburn unless that is explicit in the API.
- Existing SDC plotfile/reaction-source behavior remains unchanged after the refactor.
Summary
construct_old_react_source()takesU_stateandR_source, but in the 4th-order averaged-input path it also writes centered reaction data into the class memberSburn. That side effect is not visible in the interface and makes the function unsafe to reuse without knowledge of internal scratch-state conventions.Affected code
Problem
In the
sdc_order == 4 && input_is_averagebranch, the function computes centered reaction rates and then stores them intoSburn:Sburn[mfi].copy(R_center, obx, 0, obx, 0, NUM_STATE);The function signature and header comment only advertise output through
R_source, not throughSburn. That means the routine has a hidden dependency on mutable object state and a hidden output channel.Why this matters
R_source.Sburnto still contain an initial guess or scratch state after the call can be broken implicitly.Current risk
The current call sites appear to rely on or at least tolerate this behavior, so this may not be an immediate user-visible bug today. But it is a maintenance hazard and an easy source of future regressions.
Suggested fix
Choose one of these and make it explicit:
Sburn.R_sourcePossible acceptance criteria
Sburnunless that is explicit in the API.