RampInterpolator: potential double-counting of first stage duration in no-init path

[bernalde]

Bug Description

In RampInterpolator.__init__() (functions.py, lines 285-298), the no-init code path (i.e., when the rampspec dict does not contain an "init" key) initializes times with dt_setpt[0]:

times = np.array([0.0, self.dt_setpt[0] / constant.hr_To_min])

Then, in the count_ramp_against_dt=True loop, the first iteration (i=1) consumes dt_setpt[min(len-1, 0)] — which is again dt_setpt[0]:

for i in range(1, len(self.setpt)):
    totaltime = self.dt_setpt[min(len(self.dt_setpt)-1, i-1)] / constant.hr_To_min
    ...
    times = np.append(times, [ramptime, holdtime])

This means dt_setpt[0] is consumed twice: once during initialization (line 287) and once in the loop (line 293). For multi-setpoint schedules without an "init" key, this produces incorrect cumulative self.times and can create non-monotonic time arrays if holdtime goes negative.

Reproducer

import numpy as np
from lyopronto import functions, constant

# Multi-setpoint schedule without "init"
rampspec = {"setpt": [0.1, 0.2], "dt_setpt": [60, 60], "ramp_rate": 1.0}
ramp = functions.RampInterpolator(rampspec)

print("times:", ramp.times)
print("values:", ramp.values)
# dt_setpt[0] = 60 min = 1 hr appears twice:
#   once in initial times = [0.0, 1.0]
#   once in loop at i=1: totaltime = 1.0
# Resulting cumulative times double-count the first stage

Current Impact

Low — In practice, the no-init path is currently only used for Pchamber dicts which have a single setpoint (e.g., {"setpt": [0.15], "dt_setpt": [1800.0], "ramp_rate": 0.5}). With len(setpt) == 1, the loop range(1, 1) never executes, so the double-counting is never triggered. However, this would become a bug if:

• A user creates a multi-setpoint pressure schedule without an "init" key
• The no-init path is used for other schedule types in the future

Suggested Fix

Normalize the no-init path by either:

1. Prepending an implicit init equal to the first setpoint (matching what the init path does), so both branches share the same loop logic
2. Starting the loop at i=2 in the no-init path (to avoid re-consuming dt_setpt[0])
3. Not pre-populating dt_setpt[0] in the initial times array for the no-init path, and letting the loop handle all stage durations

Additionally, when holdtime < 0 (i.e., ramptime > totaltime), consider clamping holdtime = 0 or raising an error, rather than just warning — since negative hold times produce non-monotonic cumulative time arrays.

Found During

Code review of SECQUOIA/LyoPRONTO PR #5 (sync with upstream).

[bernalde]

Additional issue: ramp-time warning condition is too aggressive

Related to the same block of code — the validation check on line 296 uses the wrong condition:

holdtime = totaltime - ramptime
if ramptime > holdtime:
    warn(f"Ramp time from {self.setpt[i-1]:.2e} to {self.setpt[i]:.2e} exceeds total time for setpoint change, {totaltime}.")

Problem: ramptime > holdtime is equivalent to ramptime > totaltime / 2. This triggers the warning whenever the ramp takes more than half the allotted time — but the warning message says "exceeds total time," implying the ramp cannot physically complete.

Expected behavior: The warning should fire when the ramp actually cannot complete within the allotted time, i.e., when holdtime < 0 (equivalently ramptime > totaltime).

Suggested fix:

if holdtime < 0:
    warn(f"Ramp time ({ramptime:.4f} hr) from {self.setpt[i-1]:.2e} to {self.setpt[i]:.2e} "
         f"exceeds total allotted time ({totaltime:.4f} hr) for setpoint change.")
    holdtime = 0  # clamp to avoid non-monotonic times