-
Notifications
You must be signed in to change notification settings - Fork 438
Parser optimizer rewrites f(x)/g(x) assuming g is pow, breaks max() and other F2 functions #4865
Description
Summary
AMReX parser optimization incorrectly transforms divisions by any 2‑arg function with a numeric second argument as if it were pow(…, n). This breaks expressions like rb2 / max(r2, rb2) by rewriting to max(r2, -rb2) and turning division into multiplication, leading to wrong results.
This is critical to fix for our runs on Frontier.
Minimal reproducer
#include <AMReX.H>
#include <AMReX_Parser.H>
#include <AMReX_Print.H>
#include <string>
int main(int argc, char** argv) {
amrex::Initialize(argc, argv);
{
std::string expr =
"x1=x*1.0e-22; y1=y*1.0e-22; z1=z*1.0e-22; r2=x1*x1+y1*y1+z1*z1; "
"rb2=3.08567758*3.08567758; base_density_floor * (rb2 / max(r2, rb2))";
amrex::Parser parser(expr);
parser.registerVariables({"x","y","z","base_density_floor"});
amrex::Print() << "Parser AST:\n";
parser.print();
auto exe = parser.compile<4>();
double base = 1.0e-28;
amrex::Print() << "center: " << exe(0.0,0.0,0.0,base) << "\n";
amrex::Print() << "edge: " << exe(1.8369424389153397e24,0.0,0.0,base) << "\n";
}
amrex::Finalize();
return 0;
}
Observed output (trimmed):
Parser AST:
...
mul
number: 9.5214061277146573
mul
symbol: base_density_floor
max
symbol: r2
number: -9.5214061277146573
center: 0
edge: 3.212862841e-23
Expected:
The AST should preserve max(r2, rb2) with positive rb2. The expression should evaluate to base_density_floor at r=0 (not 0), and decrease with radius.
Root cause
In extern/amrex/Src/Base/Parser/AMReX_Parser_Y.cpp, the optimizer has this rule in the PARSER_DIV case:
else if (node->r->type == PARSER_F2 &&
((struct parser_f2*)(node->r))->r->type == PARSER_NUMBER)
{ // f(.) / pow(x,n) => f(.) * pow(x,-n)
node->type = PARSER_MUL;
parser_set_number(((struct parser_f2*)(node->r))->r,
-parser_get_number(((struct parser_f2*)(node->r))->r));
parser_ast_optimize(node,local_consts);
}
It does not check that ftype == PARSER_POW, so it incorrectly rewrites any F2 function with a numeric second argument (e.g., max, min, atan2, etc.).
Suggested fix
Add a guard to only apply that transformation when the function is PARSER_POW.