Combining systems compares the variables, and a loaded system holds the live ones

CHANGELOG.md

@@ -131,6 +131,15 @@ the identity of the algorithm that answered it (#420).
 
 ### Added
 
+- **A node compares by what it is** (ADR-0068).  `Variable('x') == Variable('x')` is true, and
+  so is `x*y + 1 == 1 + y*x`: nodes are interned, so equal expressions are one node, and `==`,
+  `!=` and `hash` now compare the node.  They used to compare the Python wrapper, which is a
+  new object on every call, so two handles on the same variable said they differed.  Nodes
+  work as dictionary keys and in sets, and a `VariableGroup` compares equal to a list of the
+  same variables.  Comparing a node with something that is not a node is false.
+- **A system that declares no variables takes the variables of the one it is combined with**
+  (ADR-0068): functions with nothing said about variables can be concatenated onto, or
+  blended with, a system that has them.  `AdoptVariableStructure` in C++.
 - **bertini's list containers compare by value.**  `sys.degrees() == [2, 2]` is now true when
   the degrees are 2 and 2; before, every container's `==` was object identity, so it was false
   -- as was comparing two calls of `sys.degrees()` -- and a check had to be written
@@ -330,6 +339,15 @@ the identity of the algorithm that answered it (#420).
 
 ### Changed
 
+- **Combining systems compares the variables** (ADR-0068).  `concatenate`, the homotopy
+  builders (`straight_line_homotopy`, and `MakeHomotopy` / `MakeMovingHomotopy` beneath it) and
+  `+` on systems now all make one check: the two systems must be over the same variables, in
+  the same groups, in the same order.  The homotopy builders and `+` used to compare counts
+  only, and evaluate their operands by position, so moving rows over `(y, x)` blended with a
+  system over `(x, y)` were accepted and evaluated with the coordinates exchanged: `y - 3` at
+  `(x, y) = (7, 11)` came out as 4.  Such a combination is now refused, and the message names
+  both structures and says whether the order, the grouping or the variables differ.  Code
+  that relied on matching by position has to put its variables in one order.
 - **The tracker's own settings are a config** (#457).  The predictor, the tracking tolerance and
   the path truncation threshold were bare members of the tracker, each with its own setter and
   getter and no config struct.  They were therefore absent from `get_settings`/`set_settings`, from
@@ -431,6 +449,19 @@ the identity of the algorithm that answered it (#420).
 
 ### Fixed
 
+- **A copied or unpickled system combines with its original** (ADR-0068).  `copy.deepcopy`,
+  `copy.copy` and `pickle` go through an archive, and loading from one built variables
+  outside the variable factory: a copy was over fresh variables with the same names, and
+  `concatenate(system, copy.deepcopy(other))` was refused for "differing variable
+  orderings".  A `System` and a `Slice` now re-intern what they hold as the last step of
+  loading, so a loaded system is over the same variables as every other system that names
+  them.  `straight_line_homotopy(..., fixed=...)` accepts deep-copied operands as a result;
+  it had refused what the builder beneath it accepted.
+- **A moved or assigned system keeps its auxiliary coordinates.**  `System`'s `swap`, which
+  move construction and assignment go through, left out the auxiliary declarations, so a
+  system that was moved or assigned was judged on every coordinate again.  It did not show
+  while every function returning a `System` had a single return statement, because the
+  compiler elided the move.
 - **The Cauchy endgame honours the security ceiling as the power series endgame does.**  At
   security level 0, two consecutive endpoint approximations whose largest coordinate is above
   `max_norm` truncate a path with `SecurityMaxNormReached`.  The power series endgame asks

CLAUDE.md

@@ -122,7 +122,7 @@ The project has three layers, built in order:
 
 1. **`core/`** -- C++ shared library (`libbertini2`). Header-only-heavy design under `core/include/bertini2/`. Key subsystems:
    - `function_tree/` -- Expression tree (nodes, operators, symbols) for *building and representing* polynomial systems. Nodes no longer evaluate: node-level recursive evaluation was removed -- the **SLP (`straight_line_program`) is the sole evaluator** (compile a system once, evaluate the compiled program). `Function`/`Handle` are gone; `NamedExpression` is the sole root node. See ADR-0027 (SLP: immutable Program + per-thread Memory) and ADR-0028 (named-node taxonomy).
-   - `system/` -- Polynomial system construction, start systems (`start/total_degree_linear_product.hpp`, `start/mhom.hpp`), patches, slices
+   - `system/` -- Polynomial system construction, start systems (`start/total_degree_linear_product.hpp`, `start/mhom.hpp`), patches, slices.  **Variables are canonical by name** (`Variable::Make` interns), and every operation that combines systems (`Concatenate`, `MakeHomotopy`, `MakeMovingHomotopy`, `+=`) makes ONE check, `CheckVariableStructuresMatch`: same variables, same groups, same order.  Do not compare counts, and do not give an operation its own comparison.  A `System` or `Slice` loaded from an archive re-interns itself as the last step of the load, so copies and unpickled systems recombine with their originals; a new archivable class that holds nodes needs the same step.  See ADR-0068.
    - `trackers/` -- Path tracking (fixed-precision and adaptive-precision trackers, predictors, Newton correctors)
    - `endgames/` -- Power series and Cauchy endgames for singular endpoint handling
    - `nag_algorithms/` -- Higher-level algorithms (zero-dim solve; numerical irreducible decomposition is *framework scaffolding* -- not yet implemented, its `Solve()` throws)

core/include/bertini2/function_tree/node.hpp

@@ -461,9 +461,10 @@ class Node : public VisitableBase<>, public std::enable_shared_from_this<Node>
     the just-built candidate is discarded.  Nodes whose IsSame() is identity (e.g. Variable,
     Function, Pi, E) never match, so they pass through unchanged -- no special-casing.
 
-    Thread note: guarded by a mutex, contended only during single-threaded authoring;
-    deserialization (Clone) constructs nodes WITHOUT going through Make/Intern, so per-thread
-    tracking clones stay private and un-interned.
+    Thread note: guarded by a mutex, contended only during single-threaded authoring.
+    Deserialization constructs nodes WITHOUT going through Make/Intern; whatever holds the
+    loaded nodes re-interns them as the last step of its load (System and Slice do, ADR-0068),
+    so nothing a caller receives is outside this table.
     */
     std::shared_ptr<Node> Intern(std::shared_ptr<Node> const& candidate);
 

core/include/bertini2/system/slice.hpp

@@ -44,6 +44,7 @@ needs.
 #include <vector>
 
 #include "bertini2/function_tree.hpp"
+#include "bertini2/function_tree/reintern.hpp"
 #include "bertini2/num_traits.hpp"
 #include "bertini2/eigen_extensions.hpp"
 #include "bertini2/system/blocks/linear_forms_block.hpp"
@@ -506,6 +507,14 @@ namespace bertini {
             ar & block_;
             ar & sliced_vars_;
             ar & is_homogeneous_;
+
+            // a loaded slice's variables join the live node universe, as a loaded System's do
+            // (ADR-0068): otherwise its system of linear forms could not be combined with the
+            // system it slices
+            if (Archive::is_loading::value)
+                for (auto& v : sliced_vars_)
+                    if (v)
+                        v = std::static_pointer_cast<node::Variable>(node::Reintern(v));
         }
 
         friend std::ostream& operator<<(std::ostream&, Slice const&);

core/include/bertini2/system/system.hpp

@@ -818,6 +818,12 @@ namespace bertini {
          */
         VariableGroup const& HomogenizingVariables() const { return homogenizing_variables_; }
 
+        /**
+         Get the ungrouped variables: those declared to the system but placed in no affine and
+         no projective group.
+         */
+        VariableGroup const& UngroupedVariables() const { return ungrouped_variables_; }
+
         /**
         Get the total number of variable groups in the system, including both affine and homogenous.  Ignores the ungrouped variables, because they are not in any group.
         */
@@ -1224,7 +1230,8 @@ namespace bertini {
         (derivatives, SLPs, the ordering cache) are dropped and recompute on demand.
 
         Content is unchanged (the digest is invariant), so this is allowed on a sealed
-        system.  Prefer LoadSystemUnified, which packages load -> reintern -> intern.
+        system.  Loading a System from an archive calls this itself (ADR-0068), so a caller
+        rarely needs to; LoadSystemUnified goes one step further and interns the System.
 
         \param memo The pass's memo; share it when re-interning several loaded objects.
         */
@@ -2470,6 +2477,17 @@ namespace bertini {
             // ar & std::get<Vec<complex_dbl>>(current_variable_values_);
             // ar & std::get<Vec<complex_mp>>(current_variable_values_);
 
+            // A loaded system joins the live node universe (ADR-0068).  An archive builds nodes
+            // through their default constructors, never through Make, so what comes out is
+            // content-equal to the original and shares none of its nodes: its x is not THE x,
+            // and it could not be combined with the system it was copied from.  Every load
+            // goes through here -- pickle, copy, deepcopy, the MPI broadcast, the records
+            // loader -- so the repair is made here rather than asked of each of them.
+            if (Archive::is_loading::value)
+            {
+                node::ReinternMemo memo;
+                ReinternNodes(memo);
+            }
         }
 
 
@@ -2537,13 +2555,21 @@ namespace bertini {
     /**
     \brief Throw unless two systems describe points the same way, so that combining them is meaningful.
 
-    Checks the variable structure -- variable count, homogenizing-variable count, variable-group
-    count -- and, when both are patched, that they carry the same patch.  An unpatched system is
-    free to adopt the other's, which is what `System+=System` does.
+    Checks that the two are over THE SAME VARIABLES: the same variable objects, in the same
+    groups, in the same order -- and, when both are patched, that they carry the same patch.
+    An unpatched system is free to adopt the other's, which is what `System+=System` does.
+    Variables are canonical by name, so "the same object" and "the same name" are one
+    statement, and a system that came out of an archive satisfies it (ADR-0068).
+
+    It compares the variables, not how many there are.  Two systems over `(x, y)` and
+    `(y, x)` agree in every count and mean different things by a point; a blend of them
+    evaluates one of them with its coordinates exchanged, and nothing downstream notices.
+    The same goes for combining a projective system with an affine one, or two projective
+    systems on different patches.  This is the check that notices, and every operation that
+    combines systems makes it: `Concatenate`, `MakeHomotopy`, `MakeMovingHomotopy`, `+=`.
 
-    Combining a projective system with an affine one, or two projective systems on different
-    patches, produces something whose ends do not correspond; the shapes still agree and the
-    tracking still runs, so nothing downstream notices.  This is the check that notices.
+    A system that declares no variables at all is not checked by those operations: it takes
+    the other's variable structure (see AdoptVariableStructure).
 
     \param a The first system.
     \param b The second system.
@@ -2556,6 +2582,19 @@ namespace bertini {
                                       std::string const& operation,
                                       std::string const& a_name, std::string const& b_name);
 
+    /**
+    \brief Give a system that declares no variables the variable structure of another.
+
+    The groups, their order, and the homogenizing variables are taken from `from`.  A path
+    variable `into` already has is kept.  This is what the combining operations do with an
+    operand that says nothing about variables: it is taken to be over the other operand's.
+
+    \param into The system to receive the structure; it must declare no variables.
+    \param from The system whose variable structure is taken.
+    \throws std::runtime_error If `into` already declares variables.
+    */
+    void AdoptVariableStructure(System& into, System const& from);
+
     /**
     \brief Form a homotopy that moves ONLY some rows, leaving the rest fixed and evaluated once.