Constant initializers may call the intrinsics both sides fold identically - #22
Constant initializers may call the intrinsics both sides fold identically#22chenyueqi wants to merge 3 commits into
Conversation
…ny, real, dble and int CLUBB's constants_clubb declares eps = max( 1.0e-10_core_rknd, epsilon(pi) ) and rt_tol, chi_tol the same way; penta_bicgstab_solver declares bicgstab_tol = max( 1.e-10, epsilon(bicgstab_tol) ), a kind inquiry on the constant itself. Every unit that use-imports one of them was refused at the transform. Expr gains a call kind for the intrinsics both languages fold to the same bits -- selection, correctly rounded square root, the kind's own constants, conversions -- and refuses exp, log and the trigonometrics, which gfortran folds with MPFR and libm need not match. A self-referential kind inquiry is substituted by a 64-bit literal, which is the kind the fold renders every real as; a kind= argument naming any other kind refuses. The integer-division heuristic moves from a text search for np.float64( to the tree: with_integer_division marks a quotient // from the operands' inferred types, falling back to the whole initializer's when a name leaves it open, so int(7.9) / 2 is 3 and not 3.5. The expr import in fortran/tree.py stays lazy: test_contract imports the engine with fparser blocked. Closes #16. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cne4hrGgYqhTMzVgzJtXYd
… an argument The module-parameter fold (recast.fortran.constants) is a separate path from the use-import fold and refused CLUBB's bicgstab_tol = max( 1.e-10_core_rknd, epsilon(bicgstab_tol) ) twice over: an argument's tokens could not hold a call, and the kind inquiry's argument is a name nothing defines yet. An argument is now parsed with the same rule as the expression, so a call nests; and a kind inquiry whose argument resolves to nothing is kept with no argument, which is what the target renders for it in any case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cne4hrGgYqhTMzVgzJtXYd
f5b2046 to
07c6dae
Compare
Constant folding: main's aliases for a subprogram's own parameters and this branch's intrinsic calls and kind inquiries, together; the nested-call path hands the aliases through as well. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cne4hrGgYqhTMzVgzJtXYd
chenyueqi
left a comment
There was a problem hiding this comment.
Review of #22 against two questions: is every change a generic language rule, and does anything change an existing rule's behavior. Summary:
Genericity: the intrinsic whitelist, Expr("call"), the typed integer-division decision and python_call are language rules. One leak: KINDS_64 hard-codes core_rknd (CLUBB) and shr_kind_r8 (CESM) inside the engine frontend, the first domain identifiers in src/ (main has none). Details inline.
Existing rules: the integer-division change only alters outputs that were wrong on main (e.g. 1.0 + 3/2 was 2.5, now 2.0; nrk = runge_kutta_type/10 unchanged). Two places now accept what main refused, both around kind: epsilon/huge/tiny of any undefined name in the token frontend, and every real folding as 64-bit regardless of declared kind. One test modified (test_an_initializer_too_rich_to_model_refuses, sqrt -> exp), none deleted.
Asks before merge: move the kind-name list out of the engine (or resolve the kind parameter's value from the tree), and either refuse non-64-bit declared kinds in the fold or state the assumption where the docstring currently says both sides fold "to the same value".
| KIND_INQUIRIES = frozenset({"epsilon", "huge", "tiny"}) | ||
| """Calls whose argument contributes its kind, not its value.""" | ||
|
|
||
| KINDS_64 = frozenset({"8", "dp", "r8", "core_rknd", "shr_kind_r8", "real64"}) |
There was a problem hiding this comment.
core_rknd and shr_kind_r8 are domain kind names (CLUBB's and CESM's); main has no CLUBB/CESM identifier anywhere under src/, and architecture.md ("Engine, extension, product") puts a domain table like this in the extension. It is also not a fact: CLUBB's core_rknd = CLUBB_REAL_TYPE comes from the preprocessor and can be 4. The engine already has a channel for this (kind_assumptions in frontend.py), and in the resolved tree the kind is itself a constant (integer, parameter :: r8 = 8 in this PR's own test) that use.resolve could resolve and compare to 8 instead of matching spellings.
| if name in _KIND_INQUIRIES: | ||
| # The argument names nothing yet defined -- the constant itself, | ||
| # legally -- and only its kind was ever asked for. | ||
| return {"t": "call", "v": name, "args": []}, end + 1 |
There was a problem hiding this comment.
This accepts epsilon/huge/tiny of any name not in known_names, not only the constant being declared: _intrinsic_call has no way to know which constant that is, so the comment's "the constant itself, legally" is not checked. On main the same input returned None (skipped, fail-closed). real(4) :: x followed by real(8), parameter :: e = epsilon(x) now folds to 2.2e-16 where gfortran gives 1.19e-7. Suggest passing the declared name down and refusing anything else.
| for item in spec.items if spec is not None else (): | ||
| if isinstance(item, f03.Actual_Arg_Spec): | ||
| keyword, value = (str(c).lower() for c in item.children) | ||
| if keyword == "kind" and fname in {"real", "dble", "int"} and value in KINDS_64: |
There was a problem hiding this comment.
The explicit single-precision spelling real(x, kind=sp) is refused here (good), but a bare real(pi_r8) with no kind, which also means default/single precision in Fortran, passes through as np.float64(PI_R8) and keeps full double precision where gfortran rounds. The gate is applied to the explicit form and skipped for the implicit one. Also huge/tiny are typed real and spelled np.finfo(...), so huge(0) (legal, integer result) renders np.finfo(0).max and raises at import; loud, but the whitelist admits it.
| reference carries the constant's kind and nothing else, and the fold | ||
| renders reals as 64-bit, so a 64-bit literal stands in for it. | ||
| """ | ||
| if expr.kind == "name" and expr.text == name: |
There was a problem hiding this comment.
The docstring says "the one legal self-reference" (inside a kind inquiry) but the substitution replaces every reference to name anywhere in the tree, and always with a 64-bit literal regardless of the declared kind that harvest had in hand. real(4), parameter :: tol = max(1e-10, epsilon(tol)) folds to the 64-bit epsilon. The fail-closed form would be to substitute only under KIND_INQUIRIES and raise otherwise.
|
The places where this branch turns a refusal on main into a silent fallback are recorded apart from the review, in #32 (rows 1-4), so each is ruled on rather than merged by omission. A row there closes when the rule refuses again, records what it assumed in the evidence, or takes the assumption from the extension's configuration. |
Closes #16. Found by the CLUBB extension:
constants_clubbinitializeseps,rt_tol,chi_tolwithmax( …, epsilon(pi) ), which refused every unit that imports them;penta_bicgstab_solverdeclaresbicgstab_tol = max( 1.e-10_core_rknd, epsilon(bicgstab_tol) ).Two folds, two fixes:
Use-imported constants (
recast.fortran.expr, the tree-mode path)Exprkindcallover a whitelist (max min abs sqrt epsilon huge tiny real dble int); transcendentals still refuse, with the reason in the docstring.tol = max( 1.e-10, epsilon(tol) ), is substituted by a 64-bit literal.python_callspells the call for the NumPy constants module and for the frontend's extent evaluator (no NumPy there).with_integer_division) instead of a text search, soint(7.9) / 2folds to 3.exprstays a lazy import infortran/tree.py(contract test with fparser blocked).A module's own parameters (
recast.fortran.constants, the per-file path)Full suite green (
PYTHONPATH=src pytest -q tests).🤖 Generated with Claude Code
https://claude.ai/code/session_01Cne4hrGgYqhTMzVgzJtXYd