Skip to content

construct_old_react_source() silently overwrites Sburn in the O4 averaged-input path #3384

Description

@zingale

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:

  1. Remove the side effect and return/store the centered reaction source through an explicit output argument.
  2. Rename the function or document clearly that it also populates Sburn.
  3. 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions