follow up to PR#305

Author: arnog

src/api.md

@@ -342,6 +342,39 @@ The rules used by `.simplify()` when no explicit `rules` option is passed.
 
 <MemberCard>
 
+##### ExpressionComputeEngine.solveRules
+
+```ts
+solveRules: Rule[];
+```
+
+The rules used by `solve()` to find roots of univariate expressions.
+ Each rule matches a normalized equation `f(_x) = 0` — the unknown is
+ the wildcard `_x` — and `replace` produces a root expression.
+ Conditions should reject matches where other wildcards capture `_x`.
+ Candidate roots are validated against the original equation, so an
+ over-eager template degrades to a no-op rather than a wrong answer.
+ Initialized to the built-in root-finding rules; `push()` to extend,
+ assign to replace.
+
+</MemberCard>
+
+<MemberCard>
+
+##### ExpressionComputeEngine.harmonizationRules
+
+```ts
+harmonizationRules: Rule[];
+```
+
+The rules used by `solve()` to transform an equation into equivalent,
+ easier-to-solve forms before root-finding (e.g. `ln f(x) → f(x) - 1`).
+ Same conventions and extension pattern as `solveRules`.
+
+</MemberCard>
+
+<MemberCard>
+
 ##### ExpressionComputeEngine.strict
 
 ```ts
@@ -1985,6 +2018,8 @@ type ReplaceOptions = {
   matchPermutations: boolean;
   iterationLimit: number;
   canonical: CanonicalOptions;
+  form: FormOption;
+  direction: "left-right" | "right-left";
 };
 ```
 
@@ -7508,26 +7543,29 @@ Transform the expression by applying one or more replacement rules:
 
 - If no rules apply, return `null`.
 
-See also `expr.subs()` for a simple substitution of symbols.
-
-Procedure for the determining the canonical-status of the input expression and replacements:
+The `form` option controls the form of *replacements*. The deprecated
+`canonical` option is also accepted for backward compatibility; only one
+of the two may be specified.
 
-- If `options.canonical` is set, the *entire expr.* is canonicalized to this degree: whether
-the replacement occurs at the top-level, or within/recursively.
+When neither `form` nor `canonical` is specified, the form of each
+replacement is determined as follows:
+1. the form of the replacement produced by the rule, if it has a
+   non-`'raw'` form;
+2. otherwise, the form of the expression being replaced;
+3. otherwise, the replacement is left in its raw form.
 
-- If otherwise, the *direct replacement will be canonical* if either the 'replaced' expression
-is canonical, or the given replacement (- is a Expression and -) is canonical.
-Notably also, if this replacement takes place recursively (not at the top-level), then exprs.
-containing the replaced expr. will still however have their (previous) canonical-status
-*preserved*... unless this expr. was previously non-canonical, and *replacements have resulted
-in canonical operands*. In this case, an expr. meeting this criteria will be updated to
-canonical status. (Canonicalization is opportunistic here, in other words).
+While the form applies directly to replaced sub-expressions only, a
+non-`'raw'` form also propagates 'opportunistically' up the expression
+tree: an expression whose operands all share a form after replacement
+assumes that form as well. (Specifying `form: 'raw'` disables this
+propagation.)
 
 :::info[Note]
-Applicable to canonical and non-canonical expressions.
+Applicable to input expressions of any form.
 
 To match a specific symbol (not a wildcard pattern), the `match` must be
 a `Expression` (e.g., `{ match: ce.expr('x'), replace: ... }`).
+
 For simple symbol substitution, consider using `subs()` instead.
 :::
 

src/compute-engine/boxed-expression/match.ts

@@ -248,8 +248,11 @@ function matchOnce(
       //
       // For commutative operators, try permutations unless matchPermutations is false
       const matchPerms = options.matchPermutations ?? true;
+      // Note: the pattern operator may have no definition, e.g. a user-defined
+      // function in a rule pattern (rules are boxed in a scope that only
+      // inherits from the system scope)
       result =
-        pattern.operatorDefinition!.commutative && matchPerms
+        pattern.operatorDefinition?.commutative && matchPerms
           ? matchPermutation(expr, pattern, substitution, options)
           : matchArguments(expr, pattern.ops, substitution, options);
 

src/compute-engine/boxed-expression/rules.ts

@@ -861,18 +861,15 @@ export function applyRule(
     // At least one operand (directly or recursively) matched: but continue onwards to match against
     // the top-level expr., test against any 'condition', et cetera.
     if (operandsMatched) {
-      // (note: so not consult the input-expr 'form' because, assuming that replaced operands assume
-      // the same form, this will be upcast in the subsequent branches.
-      // ^Another reason to avoid this, is if the form of replacements differ from the input expr.,
-      // then likely it is not the intention to preserve the form of the parent)
+      // 'options.form' applies to *replacements only* (allowing finer control
+      // of replacement operations), so the input expression's form is not
+      // consulted here. However, if all child operands share a form after
+      // replacement, 'eagerly' assume that form for this expression. (If this
+      // expression also matches at the top level below, its form may still be
+      // updated according to 'options.form'.)
+      // Check 'canonical' first: numbers may be jointly marked as structural
+      // and canonical.
       let form: FormOption = 'raw';
-      // The current policy for applying a form according to 'options.form' is for this to apply to
-      // *replacements only* (this ultimately allowing for finer control of replacement operations).
-      // ...However, if all child operands bear the same form, 'eagerly' assume this form for the
-      // present expression (if this present expression also later matches, form may be updated
-      // according to 'options.form'.)
-      //(@note: check 'canonical' first, because numbers may be jointly marked as structural and
-      //canonical).
       if (newOps.every((x) => x.isCanonical)) form = 'canonical';
       else if (newOps.every((x) => x.isStructural)) form = 'structural';
 
@@ -1082,9 +1079,9 @@ export function replace(
    * For both cases, if neither `x` nor `x2` (nor compared sub-expressions if recursive) is
    * structural or canonical, then return `false`.
    *
-   * **Warning**: will throw an error if it is determined, in case of `recursive !== false`, that
-   * `x` and `x2` are not structurally equivalent/have an identical tree/branching structure.
-   * (It is therefore the responsibility of the caller to ensure this beforehand)
+   * If `x` and `x2` turn out not to share an identical tree/branching structure (possible since
+   * `isSame()` follows symbol value bindings), they are conservatively reported as differing
+   * (return `true`).
    */
   function varyingForm(
     x: Expression,
@@ -1098,14 +1095,11 @@ export function replace(
     if (recursive === false) return false;
 
     if (isFunction(x) && isFunction(x2)) {
-      if (x.ops.length !== x2.ops.length)
-        throw new Error(
-          `'x' and 'x2' detected to not be structurally equivalent`
-        );
+      if (x.ops.length !== x2.ops.length) return true;
       if (x.nops === 0) return false;
 
       return x.ops.some((op, index) =>
-        recursive === true || (!isFunction(op) && !isFunction(x2.ops[index]))
+        recursive !== true && !isFunction(op) && !isFunction(x2.ops[index])
           ? false
           : varyingForm(op, x2.ops[index], { recursive })
       );

src/compute-engine/types-expression.ts

@@ -1252,32 +1252,31 @@ export interface Expression {
    *
    * - If no rules apply, return `null`.
    *
-   *
-   * Option *'form'* controls the form of *replacements*. The deprecated option *'canonical'* is also
-   * accepted for backward compatibility (only one of these should be specified).
-   *
-   * In the absence of a specified 'form' (or 'canonical') value, then the present policy for
-   * determining replacement expression form is either [to]:
-   *   - (a) first look to the form of the replacement, as generated by the replacement `Rule`...
-   *   and failing this (i.e. if this has no non-'raw' form), then:
-   *
-   *   - (b) consult the form of the *replaced* expression (provided that the form is *not
-   *   explicitly specified as `'raw'`*)
-   * If still then no form is determined, the expression is also left in its original, raw form.
-   *
-   * Notably, whilst this option does only apply directly to replaced sub-expressions, the present
-   * is also to attempt to 'opportunistically' propagate any non-`raw` up the expression 'tree' (but
-   * with the explicit provision of form 'raw' resulting in the omission of this behaviour).
+   * The `form` option controls the form of *replacements*. The deprecated
+   * `canonical` option is also accepted for backward compatibility; only one
+   * of the two may be specified.
+   *
+   * When neither `form` nor `canonical` is specified, the form of each
+   * replacement is determined as follows:
+   * 1. the form of the replacement produced by the rule, if it has a
+   *    non-`'raw'` form;
+   * 2. otherwise, the form of the expression being replaced;
+   * 3. otherwise, the replacement is left in its raw form.
+   *
+   * While the form applies directly to replaced sub-expressions only, a
+   * non-`'raw'` form also propagates 'opportunistically' up the expression
+   * tree: an expression whose operands all share a form after replacement
+   * assumes that form as well. (Specifying `form: 'raw'` disables this
+   * propagation.)
    *
    * :::info[Note]
-   * Applicable to input expressions of any 'form'.
+   * Applicable to input expressions of any form.
    *
    * To match a specific symbol (not a wildcard pattern), the `match` must be
    * a `Expression` (e.g., `{ match: ce.expr('x'), replace: ... }`).
    *
    * For simple symbol substitution, consider using `subs()` instead.
    * :::
-   *
    */
   replace(
     rules: BoxedRuleSet | Rule | Rule[],

src/compute-engine/types-kernel-serialization.ts

@@ -150,47 +150,51 @@ export type ReplaceOptions = {
   iterationLimit: number;
 
   /**
-   * Specify the canonical-status of _replaced_ sub-expressions.
+   * Canonical-status of replaced sub-expressions.
    *
-   * The specified canonical value/form may propagate upward to the input expression/root according
-   * to 'eager' replacement polcicy. See `replace()` documentation for details.
+   * Equivalent to `form`: `true` maps to `'canonical'`, `false` to `'raw'`,
+   * and a `CanonicalForm` (or array of them) is used as-is. Specifying both
+   * `canonical` and `form` is an error.
    *
-
-  /**
-   * @deprecated This is a legacy property: see option `form` for a wider span of forms.
+   * @deprecated Use `form` instead, which covers a wider range of forms.
    */
   canonical?: CanonicalOptions;
 
   /**
-   * `form` policy for *replaced* expressions. \
+   * The form (`'canonical'`, `'structural'`, `'raw'`, or specific canonical
+   * transforms) applied to *replaced* sub-expressions.
+   *
+   * The form does not automatically apply to the entire input expression.
+   * However, a non-`'raw'` form propagates upward through the expression tree:
+   * an expression whose operands all share a form after replacement assumes
+   * that form as well.
    *
-   * (If there is a recursive replacement -) Does not automatically apply to the entire input expression... However, the present `replace()` policy is to 'eagerly' propagate any specified replaced-expression replacement form the entire way 'up' an expression-tree.
-   * A value of
+   * To guarantee a form for the *entire* result, either ensure the input is
+   * already in the requested form before replacing, or request the form on the
+   * result after replacement (e.g. with `.canonical`).
    *
-   * If wishing to therefore ensure a the requested form for the *entire input* expression, either
-   * ensure the input is already in the requested form before any replacement, or simply request the
-   * form post-replacement.
+   * If no `form` (or `canonical`) option is specified, the form of each
+   * replacement is determined by the rule itself: see `replace()`.
    *
-   * ::Additional notes
-   * - form `'raw'` loses its applicability if the replaced expression - according to replacement mechanics - already assumes a form according to
-   * replacement rule logic. (for example if the applying rule is of type `RuleFunction` and the
-   * produced expression has a non-raw form).
+   * Note: a `'raw'` form does not undo a form the replacement already has,
+   * e.g. when a `RuleFunction` returns an expression that is already
+   * canonical.
    */
   form: FormOption;
 
-  /** *Traversal* direction (through the node 'tree') for both Rule matching & replacement.
-   * Can be significant in the production of the final, overall replacement result (if operating
-   * recursively) - if rule is a `RuleFunction` with arbitrary logic (e.g. replacements being
-   * index-based).
-   *
-   * In 'tree' data-structure traversal terminology, possible values span:
+  /**
+   * Traversal direction through the expression tree, for both rule matching
+   * and replacement:
    *
-   * - `'left-right'` reflects *post-order* traversal, (left sub-tree first; depth-descending) (LRN).
-   * - `'right-left'` reflects 'reverse' *post-order* (right sub-tree first; depth-descending) (RLN).
+   * - `'left-right'` (default): post-order traversal — left sub-tree first,
+   *   depth-first (LRN).
+   * - `'right-left'`: reverse post-order — right sub-tree first, depth-first
+   *   (RLN).
    *
-   * For both cases traversal is always depth-first, and always visits the root/input expr. last .
+   * In both cases the root (input) expression is visited last.
    *
-   * **Default** is: `'left-right'` (standard post-order)
+   * The direction is only observable for order-sensitive rules, e.g. a
+   * `RuleFunction` whose replacements depend on visit order.
    */
   direction: 'left-right' | 'right-left';
 };

test/compute-engine/rules.test.ts

@@ -240,3 +240,84 @@ describe('PR #301 cherry-picked fixes', () => {
     expect(expr.replace(rule)?.toString()).toBe('8');
   });
 });
+
+// Regression tests for PR #305: ReplaceOptions 'form' and 'direction'
+describe('ReplaceOptions form', () => {
+  const rule = { match: 'x', replace: ['Add', 1, 1] };
+
+  it("form: 'canonical' canonicalizes replacements (and result)", () => {
+    const expr = ce.parse('x + 1');
+    const result = expr.replace(rule, { recursive: true, form: 'canonical' });
+    expect(result?.isCanonical).toBe(true);
+    // Replacement (1 + 1) is canonicalized to 2, then folded: 2 + 1 → 3
+    expect(result?.toString()).toBe('3');
+  });
+
+  it("form: 'raw' preserves the replacement structure", () => {
+    const expr = ce.expr(['Add', 'x', 5], { form: 'raw' });
+    const result = expr.replace(rule, { recursive: true, form: 'raw' });
+    expect(result?.isCanonical).toBe(false);
+    expect(result?.json).toEqual(['Add', ['Add', 1, 1], 5]);
+  });
+
+  it('deprecated canonical: true behaves like form: canonical', () => {
+    const expr = ce.parse('x + 1');
+    const result = expr.replace(rule, { recursive: true, canonical: true });
+    expect(result?.toString()).toBe('3');
+  });
+
+  it('deprecated canonical: false behaves like form: raw', () => {
+    const expr = ce.expr(['Add', 'x', 5], { form: 'raw' });
+    const result = expr.replace(rule, { recursive: true, canonical: false });
+    expect(result?.json).toEqual(['Add', ['Add', 1, 1], 5]);
+  });
+
+  it('specifying both form and canonical throws', () => {
+    const expr = ce.parse('x + 1');
+    expect(() =>
+      expr.replace(rule, { form: 'canonical', canonical: true })
+    ).toThrow(/mutually exclusive/);
+  });
+
+  it('a replacement that only changes the form counts as a change', () => {
+    // The rule returns the canonical variant of a raw expression: the value
+    // is structurally the same, but the form differs. Before PR #305 this
+    // was treated as "no change" and replace() returned null.
+    const expr = ce.expr(['Multiply', 2, 'x'], { form: 'raw' });
+    const rule = {
+      match: ['Multiply', '_a', '_b'],
+      replace: (e) => e.canonical,
+    };
+    const result = expr.replace(rule);
+    expect(result).not.toBeNull();
+    expect(result?.isCanonical).toBe(true);
+  });
+});
+
+describe('ReplaceOptions direction', () => {
+  // An order-sensitive rule: each matched symbol is replaced with an
+  // incrementing counter, so the traversal order is observable.
+  const makeRule = () => {
+    let counter = 0;
+    return (e) => {
+      if (!e.symbol) return undefined;
+      counter += 1;
+      return { value: ce.number(counter), because: 'counter' };
+    };
+  };
+
+  it('left-right (default) visits operands in order', () => {
+    const expr = ce.expr(['List', 'a', 'b', 'c'], { form: 'raw' });
+    const result = expr.replace(makeRule(), { recursive: true });
+    expect(result?.json).toEqual(['List', 1, 2, 3]);
+  });
+
+  it('right-left visits operands in reverse order', () => {
+    const expr = ce.expr(['List', 'a', 'b', 'c'], { form: 'raw' });
+    const result = expr.replace(makeRule(), {
+      recursive: true,
+      direction: 'right-left',
+    });
+    expect(result?.json).toEqual(['List', 3, 2, 1]);
+  });
+});

test/compute-engine/simplify.test.ts

@@ -1337,3 +1337,22 @@ describe('AUTO PARTIAL FRACTION IN SIMPLIFY', () => {
     }
   });
 });
+
+// Regression test for PR #307: simplify() must dispatch its options
+// (including custom rules) to the operand re-simplification that runs after a
+// top-level rule fires (simplifyNonCommutativeFunction).
+describe('CUSTOM RULES APPLY TO OPERANDS OF REWRITTEN EXPRESSIONS', () => {
+  test('operands introduced by a custom rule are simplified with the custom rules', () => {
+    // Rule 1 rewrites the top level: F(a) -> G(H(a))
+    // Rule 2 only applies inside the rewritten result: H(a) -> a
+    // Before the fix, the operand re-simplification used the default rules,
+    // so H survived: F(x) simplified to G(H(x)) instead of G(x).
+    const rules = [
+      { match: ['F', '_a'], replace: ['G', ['H', '_a']] },
+      // (the replacement must be boxed: a string would be parsed as LaTeX)
+      { match: ['H', '_a'], replace: ce.symbol('_a') },
+    ];
+    const result = ce.box(['F', 'x']).simplify({ rules });
+    expect(result.json).toEqual(['G', 'x']);
+  });
+});