feat: Refactored test system - #65
Conversation
mentaljam
left a comment
There was a problem hiding this comment.
The provenance split between published and calculated cases is useful, and removing arbitrary per-case values such as rtol: 1e-12 is the right direction. However, replacing them with one repository-wide rtol = 1e-5 / atol = 1e-12 still assigns the same numerical notion of precision to physically different outputs and units. The new precision field does not solve this: it is optional, limited to published cases, and currently unused by all 137 cases.
I suggest resolving verification tolerance from a curated specs/quantities.yaml registry. Each scalar output and record field should reference a quantity identifier. Each quantity should provide a rational default for every supported unit: an absolute tolerance and, where appropriate for quantities spanning orders of magnitude, an optional relative tolerance, together with a short rationale. The effective tolerance would be max(absolute, relative * abs(expected)). A missing quantity or unit should be a validation blocker; during extraction, a new quantity should cause the extractor to ask the user, with a proposed default.
A function output may override the registry value when the source explicitly supports a different numerical accuracy or resolution; that override should carry source_location. There should be no case-level precision override. Source-reported RMSE or R² describes model performance and should not automatically become an implementation-test tolerance.
This keeps the fallback practical—no requirement to find source precision when it probably does not exist—while making it explicit, centralized, unit-aware, and physically defensible. The small allowance for floating-point noise can remain an internal comparator detail, separate from the scientific tolerance.
mentaljam
left a comment
There was a problem hiding this comment.
One remaining cross-target correctness issue:
mentaljam
left a comment
There was a problem hiding this comment.
Please separate normalized unit identity from source notation before treating spelling variants as independent quantity/unit tolerance entries.
mentaljam
left a comment
There was a problem hiding this comment.
Thanks — the unit registry now separates normalized unit identity from the notation found in a source, and the NaN comparison issue has been fixed. I still see the following changes as required before merge:
- Make the
retention_coefficient_atolerance discriminating. Its registry tolerance isabs: 0.01 kPa, while the Saxton verification value is about0.0001808 kPa. This allows zero and even some negative results to pass. Please choose a rational absolute/relative tolerance and add a guard or report that exposes tolerances greater than or equal to the magnitude of a non-zero expected value. - Enforce verification cases in the schema. Functions with
ready-for-implementationorimplementedstatus must contain at least one verification case; blocked functions may remain without cases. - Validate verification-case ID uniqueness within each function. I am not asking for strict cross-language identifier validation in the schema/generator. Document the requirements for IDs in the
ptf-extractskill and the relevant contributor/spec documentation, and let generated Rust compilation expose identifiers that are not valid Rust. - Preserve the case ID in generated failures. Use it as the pytest parameter ID and include it in C/C++ comparator diagnostics.
- Allow
source_locationfor every provenance kind, includingcalculated. A separate input-source field is not needed. - Correct the corpus report structure and metric. The implemented/ready subsections currently appear under the quantity-registry section rather than verification, and
missing_quantity_or_unit_validation_failuresis hard-coded to zero, so it is misleading unless it is computed from actual validation results. - Finish the terminology migration from golden tests. Rename remaining internal terms such as
GoldenInputandgolden_test_tokens, and update stale scientific notes/documentation. - Fix documentation defects: the
RВІtypo (intendedR²) and the statement that comparison policy is “never tuned in YAML,” which conflicts with function-level YAML overrides. - Separate the unrelated
myeni2021function addition from this verification-system refactor.
Per #66, this review does not require this PR to settle whether tolerances for one quantity must be physically equivalent across different units.
91063b9 to
b2009ac
Compare
Implemented the verification tolerance redesign.
Key changes:
All validation, formatting, linting, type checking, generated-output reproducibility, documentation, and target test suites pass.