Skip to content

feat: Refactored test system - #65

Merged
mentaljam merged 4 commits into
mainfrom
feat/verification-policies
Sep 10, 2026
Merged

feat: Refactored test system#65
mentaljam merged 4 commits into
mainfrom
feat/verification-policies

Conversation

@IsNeron

@IsNeron IsNeron commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Implemented the verification tolerance redesign.
Key changes:

  • Added a quantity-and-unit tolerance registry.
  • Annotated all specification outputs with stable quantity identifiers.
  • Added function-level source-specific tolerance overrides.
  • Removed case-level precision and global tolerance fallbacks.
  • Unified comparison semantics across Rust, Python, C, and C++.
  • Added strict validation for quantities, units, tolerances, duplicate entries, and verification-case shapes.
  • Updated corpus reporting, documentation, and the ptf-extract workflow.
  • Regenerated all affected targets and tests.
    All validation, formatting, linting, type checking, generated-output reproducibility, documentation, and target test suites pass.

@IsNeron
IsNeron requested a review from mentaljam September 2, 2026 10:25
@IsNeron IsNeron changed the title feat: Add scientific verification policies feat: Refactored test system Sep 4, 2026

@mentaljam mentaljam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread codegen/src/compile.rs

@mentaljam mentaljam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One remaining cross-target correctness issue:

Comment thread codegen/src/targets/native.rs Outdated

@mentaljam mentaljam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please separate normalized unit identity from source notation before treating spelling variants as independent quantity/unit tolerance entries.

Comment thread specs/quantities.yaml Outdated

@mentaljam mentaljam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

  1. Make the retention_coefficient_a tolerance discriminating. Its registry tolerance is abs: 0.01 kPa, while the Saxton verification value is about 0.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.
  2. Enforce verification cases in the schema. Functions with ready-for-implementation or implemented status must contain at least one verification case; blocked functions may remain without cases.
  3. 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-extract skill and the relevant contributor/spec documentation, and let generated Rust compilation expose identifiers that are not valid Rust.
  4. Preserve the case ID in generated failures. Use it as the pytest parameter ID and include it in C/C++ comparator diagnostics.
  5. Allow source_location for every provenance kind, including calculated. A separate input-source field is not needed.
  6. 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_failures is hard-coded to zero, so it is misleading unless it is computed from actual validation results.
  7. Finish the terminology migration from golden tests. Rename remaining internal terms such as GoldenInput and golden_test_tokens, and update stale scientific notes/documentation.
  8. Fix documentation defects: the RВІ typo (intended ) and the statement that comparison policy is “never tuned in YAML,” which conflicts with function-level YAML overrides.
  9. Separate the unrelated myeni2021 function 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.

@mentaljam
mentaljam force-pushed the feat/verification-policies branch from 91063b9 to b2009ac Compare September 9, 2026 19:09
@mentaljam
mentaljam dismissed their stale review September 9, 2026 19:11

All issues were resolved

@mentaljam
mentaljam merged commit c0197b3 into main Sep 10, 2026
12 checks passed
@mentaljam
mentaljam deleted the feat/verification-policies branch September 10, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants