Skip to content

Conversation

amstokely
Copy link
Contributor

@amstokely amstokely commented Oct 1, 2025

This PR introduces user-defined active windows for recurring alarms in the MPAS timekeeping module. Previously, alarms could only be defined with an anchor time and a recurrence interval. Once the anchor was reached, the alarm would ring repeatedly until the end of the clock. There was no way to restrict alarms to a bounded time range.

Previous behavior

Alarms were defined only by anchor time and interval:

A ---------------- B ---------------------- E
^                 ^                        ^
Clock start    Alarm anchor              Clock stop

Once the anchor was reached, alarms rang every interval until the clock ended. There was no mechanism to disable alarms outside of a chosen window.

New behavior

With this PR, all alarms now have an active window. If the user does not explicitly provide a start or stop time, the window defaults to the clock start and stop times, so existing behavior is preserved. This allows the same logic to be applied consistently to both alarms with user-defined windows and alarms without them.

Timeline with user-defined window:

A                B          C            D                E
|----------------|----------|------------|----------------|
^                ^          ^            ^                ^
Clock start   Alarm anchor  Window     Window stop     Clock stop
                           start

Alarms do not ring before the window opens, ring normally inside it at the specified interval, and stop ringing after the window closes, even if the clock continues. Resets and direction changes interact correctly with these boundaries.

Internal structure

Several internal helpers were added to make the logic clearer and easier to maintain:

  • mpas_is_alarm_active checks whether the current clock time is inside an alarm’s active window.
  • mpas_prev_ring_in_window verifies whether the alarm’s previous ring time occurred strictly inside the window, using open-interval semantics (start, stop).
  • mpas_time_in_interval performs the low-level interval membership check, supporting both closed [start, end] and open (start, end) intervals.

These helpers are not tested directly; instead, their correctness is validated indirectly through behavioral tests of alarms. This ensures that the implementation can be refactored without breaking tests, as long as the observable alarm behavior remains consistent.

Tests

A new test fixture (alarm_fixture_t) and suite (test_window_alarm) validate alarm behavior across 16 scenarios. These cover cases before and after the anchor, ringing at the window boundaries, ringing inside the window, leaving the window, delayed anchor times, resets, and clock direction changes.

mpas_reset_clock_alarm is invoked in several cases to confirm that the previous ring time is updated correctly. It is called at both window boundaries and inside the active window to exercise different internal bound checks that could fail in one case but not the other.

The tests focus strictly on behavior: they verify only whether alarms ring when expected, not how that behavior is implemented. This design allows the internal code to evolve without destabilizing the suite. At the same time, the tests serve as executable documentation of the alarm ringing contract, which was previously implicit. They will make future development in MPAS timekeeping more efficient, reliable, and safe.

@amstokely amstokely marked this pull request as draft October 1, 2025 20:58
@amstokely amstokely force-pushed the framework/clock_alarm_active_window branch 2 times, most recently from 7d34528 to 1b12b02 Compare October 2, 2025 23:40
@amstokely amstokely force-pushed the framework/clock_alarm_active_window branch 2 times, most recently from cdc1d7c to 58b3e85 Compare October 3, 2025 17:59
@amstokely amstokely marked this pull request as ready for review October 3, 2025 18:04
@amstokely amstokely requested a review from mgduda October 3, 2025 18:22
end function mpas_time_in_interval


logical function mpas_is_alarm_ringing(clock, alarmID, interval, ierr)
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks from the GitHub view that all of this new code should be indented by three fewer spaces.


mpas_prev_ring_in_window = mpas_time_in_interval( &
alarm%activeStartTime, alarm%activeStopTime, &
alarm%prevRingTime, is_open_interval=.true.)
Copy link
Contributor

Choose a reason for hiding this comment

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

While I can see some benefits to having a generic routine mpas_time_in_interval and to using that routine here, at some point the use of such a routine may obscure the essence of what's going on in my opinion. What happens when we eventually want half-open intervals? Which arguments to mpas_time_in_interval are the interval endpoints, and which is the time to be compared against those endpoints?

Instead, what about simply using code like this (adjust whitespace to taste) to replace the above three lines?

      mpas_prev_ring_in_window = (alarm % activeStartTime < alarm % prevRingTime &
                            .and. alarm % prevRingTime < alarm % activeStopTime)

!> within the active start and stop times of the given alarm. If so,
!> it returns `.true.`. An optional error code may also be returned.
!-----------------------------------------------------------------------
logical function mpas_is_alarm_active(clock, alarm, ierr)
Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative to passing in the clock and always querying MPAS_NOW to determine whether an alarm is active, we could instead pass in an MPAS_Time_type instance representing the actual -- or hypothetical -- current time to be considered when determining whether the alarm is active. This would make the routine more general, and it potentially leads to less code.

This commit adds active windows to recurring alarms. If users do not
provide a start or stop time, they default to the clock start and stop,
so existing behavior is unchanged. New helpers simplify the logic and
are validated indirectly through behavior-level tests, allowing future
refactoring without breaking the suite. A new test fixture and 16-case
suite verify alarm behavior across boundaries, resets, anchors, and
direction changes, documenting the alarm ringing contract and ensuring
safe future development of MPAS timekeeping.
@amstokely amstokely force-pushed the framework/clock_alarm_active_window branch from 83e24a2 to 65274f4 Compare October 23, 2025 23:02
@mgduda mgduda requested a review from jim-p-w October 23, 2025 23:32
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