From 99b2501d5d05963f1bc939385932039bafb0d0da Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Fri, 17 Jul 2026 21:45:09 +0800 Subject: [PATCH] fix: propagate na through math sign --- pineforge_codegen/codegen/base.py | 3 +- pineforge_codegen/codegen/tables.py | 13 +++- pineforge_codegen/signatures.py | 2 +- tests/test_math_sign_na_codegen.py | 114 ++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 tests/test_math_sign_na_codegen.py diff --git a/pineforge_codegen/codegen/base.py b/pineforge_codegen/codegen/base.py index ea72972..65cec40 100644 --- a/pineforge_codegen/codegen/base.py +++ b/pineforge_codegen/codegen/base.py @@ -63,6 +63,7 @@ MATRIX_SORT_ALLOWED_GENERIC_ELEMS, MATH_FUNC_MAP, STR_FUNC_MAP, + _MATH_SIGN_CPP_FN, _merge_kwargs, ) @@ -2939,7 +2940,7 @@ def _is_omitted_udt_field(self, node) -> bool: "exp": "std::exp", "pow": "std::pow", "sin": "std::sin", "cos": "std::cos", "tan": "std::tan", "asin": "std::asin", "acos": "std::acos", "atan": "std::atan", - "sign": "(double)([] (double _v) { return (_v>0) - (_v<0); })", + "sign": _MATH_SIGN_CPP_FN, } # Pine logical operators (word form) -> C++ operator, used when rendering diff --git a/pineforge_codegen/codegen/tables.py b/pineforge_codegen/codegen/tables.py index 4b9ab9d..8dbfa45 100644 --- a/pineforge_codegen/codegen/tables.py +++ b/pineforge_codegen/codegen/tables.py @@ -683,6 +683,17 @@ def _math_minmax_na_expr(func_name: str, args: list[str]) -> str: ) +# Pine ``math.sign`` returns a float, propagates numeric ``na``, and must not +# evaluate its argument more than once. Keep the callable separate from the +# call template because the stable-runtime TA reset fallback also substitutes +# bare ``math.sign`` member text with this same C++ function expression. +_MATH_SIGN_CPP_FN = ( + "([](const auto _pf_sign_v) -> double { " + "return is_na(_pf_sign_v) ? na() " + ": (double)((_pf_sign_v > 0) - (_pf_sign_v < 0)); })" +) + + MATH_FUNC_MAP = { "abs": "std::abs", "max": "std::max", "min": "std::min", "ceil": "std::ceil", "floor": "std::floor", "round": "std::round", @@ -691,7 +702,7 @@ def _math_minmax_na_expr(func_name: str, args: list[str]) -> str: "sin": "std::sin", "cos": "std::cos", "tan": "std::tan", "asin": "std::asin", "acos": "std::acos", "atan": "std::atan", "atan2": "std::atan2({0}, {1})", - "sign": "((({0}) > 0) - (({0}) < 0))", + "sign": _MATH_SIGN_CPP_FN, "avg": "(({0} + {1}) / 2.0)", } diff --git a/pineforge_codegen/signatures.py b/pineforge_codegen/signatures.py index a8309c7..81f4727 100644 --- a/pineforge_codegen/signatures.py +++ b/pineforge_codegen/signatures.py @@ -229,7 +229,7 @@ def _math(short_name: str, *sigs: FuncSig) -> None: _sig([("x", F)], ret=I), # round(x) → int _sig([("x", F), ("precision", I)])) # round(x, n) → float _math("round_to_mintick", _sig([("x", F)])) -_math("sign", _sig([("x", F)], ret=I)) +_math("sign", _sig([("x", F)], ret=F)) _math("max", _sig([("x", F), ("y", F)]), # 2-arg _sig([("x", F), ("y", F), ("z", F)]), # 3-arg diff --git a/tests/test_math_sign_na_codegen.py b/tests/test_math_sign_na_codegen.py new file mode 100644 index 0000000..6187c6c --- /dev/null +++ b/tests/test_math_sign_na_codegen.py @@ -0,0 +1,114 @@ +"""Generated-code regressions for Pine-compatible ``math.sign`` lowering.""" + +from __future__ import annotations + +import re + +from pineforge_codegen import transpile +from pineforge_codegen.codegen import CodeGen +from tests._compile import compile_cpp + + +_PRELUDE = '//@version=6\nstrategy("math.sign regression")\n' + + +def _gen(body: str) -> str: + return transpile(_PRELUDE + body + "\n") + + +def _assignment(cpp: str, name: str) -> str: + match = re.search(rf"^\s*{re.escape(name)}\s*=.*;$", cpp, re.MULTILINE) + assert match is not None, cpp + return match.group(0) + + +def _assert_na_safe_sign(expr: str) -> None: + assert "[](const auto _pf_sign_v) -> double" in expr + assert "is_na(_pf_sign_v) ? na()" in expr + assert "(double)((_pf_sign_v > 0) - (_pf_sign_v < 0))" in expr + + +def test_math_sign_kwarg_propagates_float_na_and_evaluates_argument_once(): + cpp = _gen("float source = na\nresult = math.sign(x=source)\nplot(result)") + assignment = _assignment(cpp, "result") + _assert_na_safe_sign(assignment) + assert assignment.count("(source)") == 1 + assert "double result = 0.0;" in cpp + + +def test_math_sign_propagates_int_na_where_int_sentinel_is_representable(): + cpp = _gen("int source = na\nresult = math.sign(source)\nplot(result)") + assignment = _assignment(cpp, "result") + _assert_na_safe_sign(assignment) + assert assignment.count("(source)") == 1 + assert "double result = 0.0;" in cpp + + +def test_math_sign_series_history_expression_is_evaluated_once(): + cpp = _gen("result = math.sign(close[1])\nplot(result)") + assignment = _assignment(cpp, "result") + _assert_na_safe_sign(assignment) + assert assignment.count("_s_close[1]") == 1 + + +def test_math_sign_stateful_argument_has_single_evaluation_evidence(): + cpp = _gen( + "bump() =>\n" + " var float calls = 0.0\n" + " calls += 1.0\n" + " calls\n" + "result = math.sign(bump())\n" + "plot(result)" + ) + assignment = _assignment(cpp, "result") + _assert_na_safe_sign(assignment) + assert len(re.findall(r"\bbump(?:_cs\d+)?\(\)", assignment)) == 1 + + +def test_math_sign_security_lowering_uses_same_na_safe_callable(): + cpp = _gen( + 'result = request.security(syminfo.tickerid, "D", ' + "math.sign(close[1]))\n" + "plot(result)" + ) + assignment = next( + line for line in cpp.splitlines() + if re.search(r"_req_sec_\d+\s*=", line) and "_pf_sign_v" in line + ) + _assert_na_safe_sign(assignment) + assert len(re.findall(r"_sec\d+_hist_close\[0\]", assignment)) == 1 + + +def test_math_sign_stable_runtime_reset_fallback_is_na_safe(monkeypatch): + # Force the legacy token-substitution lane so this pins the separate + # stable-runtime math-member mapping, not only the ordinary visitor that + # the preferred reparse lane normally reuses. + monkeypatch.setattr( + CodeGen, + "_lower_reset_expr_via_visitor", + lambda self, expanded: None, + ) + cpp = _gen( + 'length = input.int(5, "Length")\n' + "result = ta.sma(close, int(math.sign(length)) + 2)\n" + "plot(result)" + ) + reset = next( + line for line in cpp.splitlines() + if "_ta_sma_" in line and "get_input_int" in line + ) + _assert_na_safe_sign(reset) + assert reset.count('get_input_int("Length", 5)') == 1 + assert "math.sign" not in reset + + +def test_math_sign_generated_cpp_compiles_for_float_int_history_and_kwarg(): + cpp = _gen( + "float float_na = na\n" + "int int_na = na\n" + "from_float = math.sign(x=float_na)\n" + "from_int = math.sign(int_na)\n" + "from_history = math.sign(close[1])\n" + "plot(from_float + from_int + from_history)" + ) + compile_cpp(cpp, label="math-sign-na-single-evaluation")