Fix RampInterpolator double-counting and calc_knownRp pressure conversion#19
Conversation
…conversion Fix two bugs: 1. RampInterpolator with count_ramp_against_dt=True and no 'init' key double-counted dt_setpt[0], and did not clamp negative holdtime. 2. calc_knownRp.dry early-return path reported pressure in Torr instead of mTorr, inconsistent with normal output path. Also fixes several test issues: - Fix self-comparison assertion in test_freezing.py - Fix hasattr() on dict (should use 'in') in optimizer tests - Add tolerance for floating-point differences in optimizer reference tests - Fix contextlib.chdir compatibility for Python < 3.11 Closes #17 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ickaser
left a comment
There was a problem hiding this comment.
Looked it over. A few suggestions, some comments should be more terse and I don't like introducing tolerances for the ramp interpolation which should not encounter much (if any) numerical noise.
|
Addressed the review in |
Summary
count_ramp_against_dt=Trueand noinitkey double-countingdt_setpt[0], and clamp negative holdtime to 0calc_knownRp.dryearly-return path reporting pressure in Torr instead of mTorrhasattr()on dict, floating-point tolerance, Python <3.11 compatibilityTest plan
test_ramp_interpolator_noinit_multisetpt_no_double_count,test_ramp_interpolator_holdtime_clamped_to_zero)TestCalcKnownRpEarlyReturn)Closes #17
🤖 Generated with Claude Code