fix(equipment): ValidatedWriter._values_equal 改用 rel_tol+abs_tol#142
Open
csp0924 wants to merge 2 commits into
Open
fix(equipment): ValidatedWriter._values_equal 改用 rel_tol+abs_tol#142csp0924 wants to merge 2 commits into
csp0924 wants to merge 2 commits into
Conversation
…loat32 round-trip 精度損失 原本 hard-coded `1e-6` *absolute* tolerance 對工業 setpoint 量級 (kW / V / Hz·1e3, 1e3~1e7 範圍) 而言比 device 端 float32 儲存精度 (LSB ≈ value × 5.96e-8) 還緊，造成 verify=True 的 float 點位 每次 round-trip 都 VERIFICATION_FAILED。 改用 math.isclose(rel_tol=1e-6, abs_tol=1e-9)： - rel_tol=1e-6 給跨量級合理相對精度 (1ppm) - abs_tol=1e-9 給近零值絕對下限，避免 0 附近 rel_tol 退化造成 false-pass Sandbox H2 場景驗證 (1e6 量級 + float32 storage，200 cycles)： - 修正前: false_fail_rate=0.995, max_delta=0.062 - 修正後: false_fail_rate=0.0, max_delta=0.062 (仍然量到 delta，但正確被容忍) 無公開 API 變更（_values_equal 為內部 staticmethod）。
There was a problem hiding this comment.
Pull request overview
This PR updates the internal float comparison logic used by ValidatedWriter write-readback verification so that large-magnitude industrial setpoints (e.g., 1e3–1e7) don’t false-fail due to overly strict absolute-only tolerance, and adds targeted regression tests for the new behavior.
Changes:
- Update
ValidatedWriter._values_equalto usemath.isclose(expected, actual, rel_tol=1e-6, abs_tol=1e-9)for float comparisons. - Add a new test module covering large-magnitude float32 round-trip behavior and
verify=Trueintegration scenarios. - Add/retain
tests/equipment/transport/__init__.py(no functional behavior introduced).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
csp_lib/equipment/transport/writer.py |
Switch float equality from hard-coded absolute tolerance to math.isclose with relative + absolute tolerances. |
tests/equipment/transport/test_writer_tolerance.py |
New regression tests for tolerance behavior (unit + integration via write(..., verify=True)). |
tests/equipment/transport/__init__.py |
Empty package marker; no functional change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+219
| - ``abs_tol=1e-9`` 作為近零值絕對下限，避免 rel_tol 在 0 附近退化造成 | ||
| 0 vs 任意小數的 false-pass。 |
Comment on lines
+9
to
+12
| # 修正後採用 math.isclose(rel_tol=1e-6, abs_tol=1e-9): | ||
| # - rel_tol=1e-6 給大數值合理相對精度 (1ppm) | ||
| # - abs_tol=1e-9 給近零值安全絕對下限, 避免 0.0 vs 1e-12 假成功 | ||
| # |
Comment on lines
+83
to
+86
| """模擬 device 端用 float32 儲存的 Modbus 客戶端。 | ||
|
|
||
| write_multiple_registers 收到的暫存器, 我們解回 float, truncate 成 float32, | ||
| 再以 float32 編碼回兩個 16-bit word, 供 read_holding_registers 回吐。 |
依 Copilot review 反饋修正 docstring/comment, 不影響行為： - writer.py:218 把「avoid false-pass」改寫為 near-zero noise floor (rel_tol 在 0 附近退化, abs_tol 提供最小絕對容差; |delta| ≤ 1e-9 視為相等, > 1e-9 仍拒絕), 避免讀者誤解 math.isclose 行為 - test_writer_tolerance.py header 同步修正 abs_tol 文案 (原本「0.0 vs 1e-12 假成功」 與實際 math.isclose 行為矛盾) - mock_client_float32_storage fixture docstring 改成貼近實作 (原本宣稱會 decode/ truncate/re-encode, 實際只是原樣保存 write 進來的 float32 registers)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
問題
Closed-loop probe 發現：
ValidatedWriter._values_equal用 hard-codedabs(expected - actual) < 1e-6absolute tolerance。工業設備設定值常落在 1e3-1e7 範圍（功率 kW/kVar、電壓 V、頻率 Hz·1e3），且 PLC 普遍以 float32 存（LSB ≈ value × 5.96e-8 → 1e6 時 ≈ 0.06）。1e-6比設備自身儲存精度還嚴，導致verify=True在工業典型量級下幾乎必 false-fail。Sandbox 證據（
sandbox/write_validation_readback_race_demo.pyH2）：value ~1.23e6、device float32 round-trip → pre-fixfalse_fail_rate=0.995, max_delta=0.062。改動
rel_tol=1e-6：1 ppm 相對精度，跨所有工業量級abs_tol=1e-9：防止 near-zero 假通過無 public API 變更（internal helper）。
驗證
tests/equipment/transport/test_writer_tolerance.py7 案（大量級 float32 round-trip / kW 範圍 / 近零 abs_tol floor / 完全相等 / 明顯不等 / 整合測試 × 2）。Pre-fix 3 FAIL → post-fix 全 PASS。false_fail_rate: 0.995 → 0.0✓