Analysis groups reject a monthly reduction when RestartWrite has no periodic alarm

[xylar]

Filed by Claude (Anthropic's Claude Code), posting through @xylar's GitHub account. The investigation, the diagnosis and the wording below are Claude's, not @xylar's. He reviewed it and asked for it to be filed. Please direct questions to him, but treat the analysis as coming from an AI agent and check it accordingly.

An analysis group configured with a monthly ReductionPeriod aborts at startup when the RestartWrite stream uses FreqUnits: OnShutdown, reporting that an interval the user never set is not divisible by the averaging period.

What happens

[critical] [AnalysisGroup.cpp:235] Analysis: The RestartWrite interval is not divisible by the averaging period, 1 Months. Currently, temporal averaging is only available over intervals where RestartPeriod % PeriodInterval == 0
[critical] [Error.cpp:43] Omega aborting

What was expected

Either the run proceeds — a run that writes a restart only at shutdown has no periodic restart interval for a reduction period to be inconsistent with — or the error names OnShutdown as the problem rather than an interval that does not exist.

Why it happens

AnalysisGroup::createAnalysisGroupStreams() validates the restart interval without first checking that RestartWrite has a periodic alarm:

auto RestartAlarm = IOStream::getAlarm("RestartWrite");
bool IsDivisible = RestartAlarm->getInterval()->isDivisibleBy(PeriodInterval);

IOStream::create() only constructs an Alarm when FreqUnits names a standard time unit. OnStartup, OnShutdown and never set a flag and leave MyAlarm default-constructed, so getAlarm() returns a valid pointer to an alarm that was never set up, and getInterval() returns a default TimeInterval — IsCalendar = false, Interval = 0 seconds.

TimeInterval::isDivisibleBy() then takes its variable-month branch, because the divisor is in months and the calendar is NoLeap. The ComparingMonthWithDayOrShorter test is satisfied through its !IsCalendar arm, and the branch returns false unconditionally for a months divisor:

if (Divisor.IsCalendar && Divisor.Units == TimeUnits::Months) {
   // Divisor is months, dividend is day-based or shorter
   return false;
}

That rule is right for a real day-based interval, which cannot divide a variable-length month evenly. It is applied here to a zero interval that stands for "no periodic restart at all", so the check never reaches the seconds comparison further down, where 0 % anything == 0 would have passed.

A day-based RestartWrite interval fails the same check, which is correct; only a Months or Years restart interval satisfies a monthly reduction.

Reproducing it

The check is in the shared base-class method, so any analysis group reaches it. GlobalStats is the smallest case — two changes to configs/Default.yml:

Omega:
  Analysis:
    GlobalStats:
      Enable: true
      ReductionPeriod: ["1Month"]   # a months reduction
  IOStreams:
    RestartWrite:
      Freq: 1
      FreqUnits: OnShutdown         # instead of the default Freq: 6, FreqUnits: months

The stock Default.yml does not hit this because RestartWrite defaults to Freq: 6, FreqUnits: months, which is a calendar-months interval and divides cleanly.

What was actually observed was this abort from the MOC group with ReductionPeriod: [1Month] and a Polaris-generated config setting RestartWrite to OnShutdown, on the branch for #481. The GlobalStats configuration above is the same code path reduced to what is already on develop — AnalysisGroup.cpp's check and TimeMgr.cpp's ComparingMonthWithDayOrShorter branch are both present there — but it has not been run as written.

Suggested fix

Skip the divisibility check when RestartWrite has no periodic alarm, rather than reading an interval that was never set.

#524 adds exactly the accessor this needs — Alarm::isPeriodic(), which returns the Periodic flag, and that flag is false for a default-constructed alarm. Once #524 lands the fix is a guard:

auto RestartAlarm = IOStream::getAlarm("RestartWrite");
if (RestartAlarm->isPeriodic()) {
   bool IsDivisible = RestartAlarm->getInterval()->isDivisibleBy(PeriodInterval);
   ...
}

#524 does not otherwise change this path: it leaves AnalysisGroup.cpp, IOStream.cpp and TimeInterval::isDivisibleBy() untouched, and does not change RestartWrite's Freq / FreqUnits. So this abort survives it.

Worth considering separately: Alarm::getInterval() returning a zero interval for a non-periodic alarm is easy to misread, and this is one caller that did.