diff --git a/CHANGELOG.md b/CHANGELOG.md index adad65759..91bf501a9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CLAUDE.md b/CLAUDE.md index c1a9e3d01..1b9c969df 100644 --- a/CLAUDE.md +++ b/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) diff --git a/core/include/bertini2/function_tree/node.hpp b/core/include/bertini2/function_tree/node.hpp index 6af174117..29d1fedcd 100755 --- a/core/include/bertini2/function_tree/node.hpp +++ b/core/include/bertini2/function_tree/node.hpp @@ -461,9 +461,10 @@ class Node : public VisitableBase<>, public std::enable_shared_from_this 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 Intern(std::shared_ptr const& candidate); diff --git a/core/include/bertini2/system/slice.hpp b/core/include/bertini2/system/slice.hpp index 75e0e3946..db6b66ffb 100644 --- a/core/include/bertini2/system/slice.hpp +++ b/core/include/bertini2/system/slice.hpp @@ -44,6 +44,7 @@ needs. #include #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::Reintern(v)); } friend std::ostream& operator<<(std::ostream&, Slice const&); diff --git a/core/include/bertini2/system/system.hpp b/core/include/bertini2/system/system.hpp index ff33019a4..6f342c39f 100644 --- a/core/include/bertini2/system/system.hpp +++ b/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>(current_variable_values_); // ar & std::get>(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. diff --git a/core/src/system/system.cpp b/core/src/system/system.cpp index 9f4af0521..0852cc9b5 100644 --- a/core/src/system/system.cpp +++ b/core/src/system/system.cpp @@ -53,6 +53,13 @@ namespace bertini swap(a.homogenizing_variables_,b.homogenizing_variables_); swap(a.pre_homogenization_functions_,b.pre_homogenization_functions_); + // swap is a hand-maintained mirror of the members, as the copy constructor is: these two + // were missing, so a System that was MOVED or assigned lost its auxiliary declarations. + // A function returning a System by value moves it whenever the copy cannot be elided, + // which is how MakeHomotopy came to hand back a homotopy judged on every coordinate. + swap(a.auxiliary_variable_groups_,b.auxiliary_variable_groups_); + swap(a.auxiliary_coordinates_,b.auxiliary_coordinates_); + swap(a.time_order_of_variable_groups_,b.time_order_of_variable_groups_); swap(a.have_path_variable_,b.have_path_variable_); @@ -1799,14 +1806,79 @@ namespace bertini throw std::runtime_error(ss.str()); }; - // homogenizing variables FIRST: a projective/affine mix also shows up as a variable-count - // difference, and "one is projective and the other is affine" is the useful diagnosis + // homogenizing variables FIRST: a projective/affine mix also shows up as a difference in + // the variables, and "one is projective and the other is affine" is the useful diagnosis if (a.NumHomVariables() != b.NumHomVariables()) complain("homogenizing variables", a.NumHomVariables(), b.NumHomVariables()); - if (a.NumVariables() != b.NumVariables()) - complain("variables", a.NumVariables(), b.NumVariables()); - if (a.NumTotalVariableGroups() != b.NumTotalVariableGroups()) - complain("variable groups", a.NumTotalVariableGroups(), b.NumTotalVariableGroups()); + + // THE VARIABLES, not how many of them there are. Counts agree for (x, y) against + // (y, x), and a blend of two such systems evaluates one of them with its coordinates + // exchanged. Variables are canonical by name and a loaded system re-interns its own + // (ADR-0068), so comparing the objects is comparing the names. + auto names = [](VariableGroup const& group) + { + std::string out; + for (auto const& v : group) + { + if (!out.empty()) + out += ", "; + out += v ? v->name() : std::string(""); + } + return out; + }; + auto describe = [&names](System const& s) + { + std::ostringstream ss; + ss << "(" << names(s.VariableOrdering()) << ")"; + auto const affine = s.VariableGroups(); + auto const projective = s.HomVariableGroups(); + if (affine.size() + projective.size() > 1 || s.NumUngroupedVariables() > 0) + { + ss << ", grouped as"; + for (auto const& g : affine) + ss << " [" << names(g) << "]"; + for (auto const& g : projective) + ss << " projective [" << names(g) << "]"; + if (s.NumUngroupedVariables() > 0) + ss << " ungrouped [" << names(s.UngroupedVariables()) << "]"; + } + return ss.str(); + }; + + auto const order_a = a.VariableOrdering(); + auto const order_b = b.VariableOrdering(); + bool const same_order = order_a == order_b; + bool const same_groups = a.VariableGroups() == b.VariableGroups() + && a.HomVariableGroups() == b.HomVariableGroups() + && a.UngroupedVariables() == b.UngroupedVariables() + && a.HomogenizingVariables() == b.HomogenizingVariables(); + if (!same_order || !same_groups) + { + std::stringstream ss; + ss << operation << " needs systems over the same variables, in the same groups and" + " the same order, but " << a_name << " is over " << describe(a) << " and " + << b_name << " is over " << describe(b) << "."; + + std::vector sorted_a, sorted_b; + for (auto const& v : order_a) sorted_a.push_back(v ? v->name() : std::string()); + for (auto const& v : order_b) sorted_b.push_back(v ? v->name() : std::string()); + bool const same_names_in_order = sorted_a == sorted_b; + std::sort(sorted_a.begin(), sorted_a.end()); + std::sort(sorted_b.begin(), sorted_b.end()); + + if (same_order) + ss << " The variables and their order agree; the grouping does not, and the" + " grouping decides how a system is homogenized and what its degrees are."; + else if (same_names_in_order) + ss << " The names agree, but the variables are different objects: one of the" + " systems holds variables that were not made by the variable factory."; + else if (sorted_a == sorted_b) + ss << " They are the same variables in a different order, so a point means" + " different things to the two systems."; + else + ss << " They are over different variables."; + throw std::runtime_error(ss.str()); + } // A patch chooses which representative of a projective point is meant, so two patched // systems carrying different patches are not describing the same points. An unpatched @@ -1819,13 +1891,48 @@ namespace bertini } + void AdoptVariableStructure(System& into, System const& from) + { + if (into.NumVariables() != 0) + throw std::runtime_error("AdoptVariableStructure: the receiving system already declares" + " variables, so there is a structure to compare and nothing to adopt."); + + // CopyVariableStructure brings the other's path variable along; a path variable the + // receiver already has is its own and stays + bool const had_path_variable = into.HavePathVariable(); + auto const own_path_variable = had_path_variable ? into.GetPathVariable() : nullptr; + into.CopyVariableStructure(from); + if (had_path_variable) + into.AddPathVariable(own_path_variable); + } + + namespace { + /// A system that says nothing about variables takes the word of the one it is combined with. + bool DeclaresNoVariables(System const& s) + { + return s.NumVariables() == 0; + } + + /// A copy of `s` over the variable structure of `from`. + System OverTheVariablesOf(System s, System const& from) + { + AdoptVariableStructure(s, from); + return s; + } + } + + System& System::operator+=(System const& rhs) { ThrowIfSealed("operator+= (append functions)"); if (this->NumTotalFunctions()!=rhs.NumTotalFunctions()) throw std::runtime_error("cannot add two Systems with differing numbers of functions"); - CheckVariableStructuresMatch(*this, rhs, "System+=System", "the left system", "the right system"); + // a side that declares no variables takes the other's; two that do must agree + if (DeclaresNoVariables(*this) && !DeclaresNoVariables(rhs)) + AdoptVariableStructure(*this, rhs); + else if (!DeclaresNoVariables(rhs)) + CheckVariableStructuresMatch(*this, rhs, "System+=System", "the left system", "the right system"); // @@ -1920,16 +2027,13 @@ namespace bertini System Concatenate(System sys1, System const& sys2) { - // first we will deal with the variable structure - if (sys1.NumVariables()!=sys2.NumVariables()) - throw std::runtime_error("concatenating systems with differing numbers of variables"); - - if (sys1.VariableOrdering() != sys2.VariableOrdering()) - throw std::runtime_error("concatenating systems with differing variable orderings"); - - if (sys1.IsPatched() && sys2.IsPatched()) - if (sys1.GetPatch()!=sys2.GetPatch()) - throw std::runtime_error("concatenating systems with incompatible patches"); + // First the variable structure: the same check every combining operation makes, on the + // variables themselves. A side that declares no variables takes the other's. + if (DeclaresNoVariables(sys1) && !DeclaresNoVariables(sys2)) + AdoptVariableStructure(sys1, sys2); + else if (!DeclaresNoVariables(sys2)) + CheckVariableStructuresMatch(sys1, sys2, "Concatenate", + "the first system", "the second system"); if (sys2.IsPatched() && !sys1.IsPatched()) sys1.CopyPatches(sys2); // give the unpatched result sys2's patch @@ -1980,6 +2084,12 @@ namespace bertini std::string const& path_variable_name, std::shared_ptr const& gamma) { + // an end that declares no variables is over the other end's + if (DeclaresNoVariables(target) != DeclaresNoVariables(start)) + return DeclaresNoVariables(target) + ? MakeHomotopy(OverTheVariablesOf(target, start), start, path_variable_name, gamma) + : MakeHomotopy(target, OverTheVariablesOf(start, target), path_variable_name, gamma); + // The two ends of a homotopy must describe points the same way, or the deformation runs // between things that do not correspond -- and nothing downstream notices, because the // shapes still agree and the tracking still runs. Same check System+=System makes. @@ -2037,6 +2147,27 @@ namespace bertini { if (start_moving.NumNaturalFunctions() != end_moving.NumNaturalFunctions()) throw std::runtime_error("MakeMovingHomotopy: start_moving and end_moving must have the same number of functions (they are the two endpoints of the moving rows)."); + + // an operand that declares no variables is over the variables of the first one that does + { + System const* declared = nullptr; + for (System const* s : {&fixed, &start_moving, &end_moving}) + if (!declared && !DeclaresNoVariables(*s)) + declared = s; + bool const some_undeclared = DeclaresNoVariables(fixed) + || DeclaresNoVariables(start_moving) + || DeclaresNoVariables(end_moving); + if (declared && some_undeclared) + { + auto over = [declared](System const& s) + { + return DeclaresNoVariables(s) ? OverTheVariablesOf(s, *declared) : System(s); + }; + return MakeMovingHomotopy(over(fixed), over(start_moving), over(end_moving), + path_variable_name, gamma); + } + } + // Same variable structure across all three, including whether they are projective and // which patch they carry: a homotopy whose ends describe points differently deforms // between things that do not correspond, and nothing downstream notices. diff --git a/core/test/classes/auxiliary_coordinates_test.cpp b/core/test/classes/auxiliary_coordinates_test.cpp index 8e5c8bf98..859a4a1d7 100644 --- a/core/test/classes/auxiliary_coordinates_test.cpp +++ b/core/test/classes/auxiliary_coordinates_test.cpp @@ -183,6 +183,31 @@ BOOST_AUTO_TEST_CASE(a_copy_is_judged_the_way_its_original_is) } +BOOST_AUTO_TEST_CASE(a_moved_or_assigned_system_is_judged_the_way_its_original_is) +{ + // swap is a hand-written mirror too, and it forgot the auxiliary declarations, so a System + // that was moved or assigned lost them. Nothing showed while every function returning a + // System had one return statement, because the compiler elided the move; MakeHomotopy gained + // a second one and handed back a homotopy with no auxiliary coordinates. + auto sys = TwoGroups(); + sys.SetAuxiliaryVariableGroups({1}); + auto const groups = sys.AuxiliaryVariableGroups(); + + System assigned; + assigned = sys; + BOOST_CHECK(assigned.AuxiliaryVariableGroups() == groups); + + System moved(std::move(assigned)); + BOOST_CHECK(moved.AuxiliaryVariableGroups() == groups); + + auto other = TwoGroups(); + other.SetAuxiliaryCoordinates({0}); + swap(moved, other); + BOOST_CHECK(other.AuxiliaryVariableGroups() == groups); + BOOST_CHECK(moved.AuxiliaryCoordinates() == std::vector{0}); +} + + BOOST_AUTO_TEST_CASE(a_homotopy_is_judged_the_way_its_target_is) { // the tracker measures the HOMOTOPY, not the system the author wrote, so auxiliary diff --git a/core/test/classes/system_identity_test.cpp b/core/test/classes/system_identity_test.cpp index d79be5a81..3b10c4684 100644 --- a/core/test/classes/system_identity_test.cpp +++ b/core/test/classes/system_identity_test.cpp @@ -40,6 +40,7 @@ regenerate the fixture in the same commit, never silently. #include #include "bertini2/system/system.hpp" +#include "bertini2/system/slice.hpp" #include "bertini2/system/start_systems.hpp" #include "bertini2/io/parsing/system_parsers.hpp" #include "bertini2/function_tree/canonical.hpp" @@ -415,18 +416,20 @@ BOOST_AUTO_TEST_CASE(reintern_nodes_preserves_digest_and_unifies_node_pointers) ia >> loaded; } - // before: content-equal but node-forked + // the load itself re-interns (ADR-0068): content-equal AND node-shared, with nothing asked + // of the caller BOOST_REQUIRE(loaded.IsSame(original)); auto const digest_before = loaded.ContentDigest(); - BOOST_CHECK(loaded.GetNaturalFunctions().front() != original.GetNaturalFunctions().front()); + BOOST_CHECK(loaded.GetNaturalFunctions().front() == original.GetNaturalFunctions().front()); + BOOST_CHECK(loaded.VariableOrdering() == original.VariableOrdering()); // shared_ptr equality per entry + // doing it again by hand changes nothing bertini::node::ReinternMemo memo; loaded.ReinternNodes(memo); - // after: digest invariant, node pointers unified with the live universe BOOST_CHECK(digest_before == loaded.ContentDigest()); BOOST_CHECK(loaded.GetNaturalFunctions().front() == original.GetNaturalFunctions().front()); - BOOST_CHECK(loaded.VariableOrdering() == original.VariableOrdering()); // shared_ptr equality per entry + BOOST_CHECK(loaded.VariableOrdering() == original.VariableOrdering()); } BOOST_AUTO_TEST_CASE(two_archives_of_one_system_load_to_one_object) @@ -486,6 +489,229 @@ BOOST_AUTO_TEST_CASE(loaded_blend_homotopy_unifies_recursively) BOOST_CHECK(values.allFinite()); } +// ---- combining systems compares the VARIABLES (ADR-0068) ---- +// +// Every operation that combines systems -- Concatenate, MakeHomotopy, MakeMovingHomotopy, += -- +// asks one question: are these over the same variables, in the same groups, in the same order? +// The homotopy builders used to count instead, and blended (x, y) with (y, x) by position. + +namespace { + +/// A system over the given variables, in the given groups, with the given functions. +System Over(std::vector const& groups, + std::vector> const& functions) +{ + System s; + for (auto const& g : groups) + s.AddVariableGroup(g); + for (auto const& f : functions) + s.AddFunction(f); + return s; +} + +/// The system after a round trip through a text archive. +System ThroughAnArchive(System const& original) +{ + std::stringstream archive_stream; + { + boost::archive::text_oarchive oa(archive_stream); + oa << original; + } + System loaded; + { + boost::archive::text_iarchive ia(archive_stream); + ia >> loaded; + } + return loaded; +} + +/// The message a callable throws, or the empty string if it does not throw. +template +std::string Refusal(F&& f) +{ + try { f(); } + catch (std::runtime_error const& e) { return e.what(); } + return ""; +} + +} // namespace + +BOOST_AUTO_TEST_CASE(a_loaded_system_recombines_with_its_original) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto circle = Over({{x, y}}, {x*x + y*y - 1}); + auto line = Over({{x, y}}, {x - y}); + + auto loaded = ThroughAnArchive(line); + BOOST_CHECK(loaded.VariableOrdering() == line.VariableOrdering()); // THE x, THE y + + auto both = bertini::Concatenate(circle, loaded); + BOOST_CHECK_EQUAL(both.NumTotalFunctions(), 2u); + + bertini::Vec point(2), values(2); + point << bertini::complex_dbl(2.0, 0.0), bertini::complex_dbl(5.0, 0.0); + both.EvalInPlace(values, point); + BOOST_CHECK_SMALL(abs(values(0) - bertini::complex_dbl(28.0, 0.0)), 1e-12); // 4 + 25 - 1 + BOOST_CHECK_SMALL(abs(values(1) - bertini::complex_dbl(-3.0, 0.0)), 1e-12); // 2 - 5 +} + +BOOST_AUTO_TEST_CASE(a_loaded_homogenized_system_recombines_with_its_original) +{ + // the cellular port's case: three copies homogenized one by one, the fixed one patched + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto fixed = ThroughAnArchive(Over({{x, y}}, {x*x + y*y - 1})); + auto start = ThroughAnArchive(Over({{x, y}}, {x - 2})); + auto end = ThroughAnArchive(Over({{x, y}}, {y - 3})); + fixed.Homogenize(); + fixed.AutoPatch(); + start.Homogenize(); + end.Homogenize(); + + BOOST_CHECK(fixed.VariableOrdering() == start.VariableOrdering()); + BOOST_CHECK_NO_THROW(bertini::MakeMovingHomotopy(fixed, start, end, "t", bertini::node::Integer::Make(1))); + BOOST_CHECK_NO_THROW(bertini::Concatenate(fixed, end)); +} + +BOOST_AUTO_TEST_CASE(a_different_order_is_refused_by_every_combining_operation) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto xy = Over({{x, y}}, {x - 2}); + auto yx = Over({{y, x}}, {y - 3}); + auto fixed = Over({{x, y}}, {x + y - 5}); + + auto const gamma = bertini::node::Integer::Make(1); + for (auto const& said : { + Refusal([&]{ bertini::Concatenate(xy, yx); }), + Refusal([&]{ bertini::MakeHomotopy(xy, yx, "t", gamma); }), + Refusal([&]{ bertini::MakeMovingHomotopy(fixed, xy, yx, "t", gamma); }), + Refusal([&]{ auto sum = xy; sum += yx; }) }) + { + BOOST_CHECK_MESSAGE(said.find("same variables in a different order") != std::string::npos, + "expected a refusal naming the order, got: '" << said << "'"); + BOOST_CHECK(said.find("(x, y)") != std::string::npos); + BOOST_CHECK(said.find("(y, x)") != std::string::npos); + } +} + +BOOST_AUTO_TEST_CASE(the_same_order_blends_the_rows_it_was_given) +{ + // the measured defect: end rows over (y, x), blended by position, evaluated y - 3 at + // (x, y) = (7, 11) as 7 - 3. Over (x, y) the row is what it says. + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto fixed = Over({{x, y}}, {x + y - 5}); + auto start = Over({{x, y}}, {x - 2}); + auto end = Over({{x, y}}, {y - 3}); + auto homotopy = bertini::MakeMovingHomotopy(fixed, start, end, "t", bertini::node::Integer::Make(1)); + + bertini::Vec point(2), values(2); + point << bertini::complex_dbl(7.0, 0.0), bertini::complex_dbl(11.0, 0.0); + homotopy.EvalInPlace(values, point, bertini::complex_dbl(0.0, 0.0)); + BOOST_CHECK_SMALL(abs(values(0) - bertini::complex_dbl(13.0, 0.0)), 1e-12); // 7 + 11 - 5 + BOOST_CHECK_SMALL(abs(values(1) - bertini::complex_dbl(8.0, 0.0)), 1e-12); // 11 - 3 +} + +BOOST_AUTO_TEST_CASE(a_different_grouping_is_refused) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"), z = Variable::Make("z"); + auto one_group = Over({{x, y, z}}, {x - 1}); + auto two_groups = Over({{x, y}, {z}}, {z - 1}); + + BOOST_CHECK(one_group.VariableOrdering() == two_groups.VariableOrdering()); // same order... + auto const said = Refusal([&]{ bertini::Concatenate(one_group, two_groups); }); + BOOST_CHECK_MESSAGE(said.find("the grouping does not") != std::string::npos, said); // ...but + BOOST_CHECK(!Refusal([&]{ bertini::MakeHomotopy(one_group, two_groups, "t", + bertini::node::Integer::Make(1)); }).empty()); +} + +BOOST_AUTO_TEST_CASE(different_variables_are_refused) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"), z = Variable::Make("z"); + auto xy = Over({{x, y}}, {x - 1}); + auto xz = Over({{x, z}}, {z - 1}); // overlapping + auto just_z = Over({{z}}, {z - 1}); // disjoint, and a different count + + BOOST_CHECK(Refusal([&]{ bertini::Concatenate(xy, xz); }).find("different variables") != std::string::npos); + BOOST_CHECK(Refusal([&]{ bertini::Concatenate(xy, just_z); }).find("different variables") != std::string::npos); + BOOST_CHECK(!Refusal([&]{ bertini::MakeHomotopy(xy, xz, "t", bertini::node::Integer::Make(1)); }).empty()); +} + +BOOST_AUTO_TEST_CASE(a_projective_system_and_an_affine_one_are_refused_with_the_useful_diagnosis) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto affine = Over({{x, y}}, {x - 1}); + auto projective = Over({{x, y}}, {y - 1}); + projective.Homogenize(); + + auto const said = Refusal([&]{ bertini::Concatenate(affine, projective); }); + BOOST_CHECK_MESSAGE(said.find("One of them is projective and the other is affine") != std::string::npos, said); +} + +BOOST_AUTO_TEST_CASE(a_system_that_declares_no_variables_takes_the_others) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto declared = Over({{x, y}}, {x*x + y*y - 1}); + auto bare = Over({}, {x - y}); // functions, and nothing said of variables + BOOST_REQUIRE_EQUAL(bare.NumVariables(), 0u); + + // either way round + auto first = bertini::Concatenate(declared, bare); + auto second = bertini::Concatenate(bare, declared); + BOOST_CHECK(first.VariableOrdering() == declared.VariableOrdering()); + BOOST_CHECK(second.VariableOrdering() == declared.VariableOrdering()); + BOOST_CHECK_EQUAL(first.NumTotalFunctions(), 2u); + BOOST_CHECK_EQUAL(second.NumTotalFunctions(), 2u); + + bertini::Vec point(2), values(2); + point << bertini::complex_dbl(2.0, 0.0), bertini::complex_dbl(5.0, 0.0); + second.EvalInPlace(values, point); + BOOST_CHECK_SMALL(abs(values(0) - bertini::complex_dbl(-3.0, 0.0)), 1e-12); // bare came first + BOOST_CHECK_SMALL(abs(values(1) - bertini::complex_dbl(28.0, 0.0)), 1e-12); + + // the homotopy builders too: moving rows given as bare functions are over the fixed system's + auto start = Over({}, {x - 2}); + auto end = Over({}, {y - 3}); + auto homotopy = bertini::MakeMovingHomotopy(declared, start, end, "t", bertini::node::Integer::Make(1)); + BOOST_CHECK(homotopy.VariableOrdering() == declared.VariableOrdering()); + homotopy.EvalInPlace(values, point, bertini::complex_dbl(0.0, 0.0)); + BOOST_CHECK_SMALL(abs(values(1) - bertini::complex_dbl(2.0, 0.0)), 1e-12); // 5 - 3 +} + +BOOST_AUTO_TEST_CASE(adopting_is_refused_when_there_is_a_structure_to_compare) +{ + using bertini::node::Variable; + auto x = Variable::Make("x"), y = Variable::Make("y"); + auto xy = Over({{x, y}}, {x - 1}); + auto yx = Over({{y, x}}, {y - 1}); + BOOST_CHECK_THROW(bertini::AdoptVariableStructure(xy, yx), std::runtime_error); +} + +BOOST_AUTO_TEST_CASE(a_loaded_slice_is_over_the_same_variables) +{ + using bertini::node::Variable; + bertini::VariableGroup vars{Variable::Make("x"), Variable::Make("y")}; + auto slice = bertini::Slice::RandomComplex(vars, 1); + + std::stringstream archive_stream; + { + boost::archive::text_oarchive oa(archive_stream); + oa << slice; + } + bertini::Slice loaded; + { + boost::archive::text_iarchive ia(archive_stream); + ia >> loaded; + } + BOOST_CHECK(loaded.Variables() == vars); // shared_ptr equality per entry +} + // ---- the golden fixture: cross-session digest stability ---- BOOST_AUTO_TEST_CASE(golden_digests_match_committed_fixture) diff --git a/docs/adr/0068-combining-systems-compares-the-variables.md b/docs/adr/0068-combining-systems-compares-the-variables.md new file mode 100644 index 000000000..fb5a6c1b3 --- /dev/null +++ b/docs/adr/0068-combining-systems-compares-the-variables.md @@ -0,0 +1,78 @@ +# ADR-0068: Combining systems compares the variables, and a loaded system holds the live ones + +**Status:** Accepted +**Date:** 2026-09-29 + +## Context + +Variables are canonical by name: `Variable::Make("x")` interns, so the `x` made here and the +`x` made there are one object. That is what makes the symbolic engine cheap, and it gives a +plain meaning to "these two systems are over the same variables": the same objects, in the +same groups, in the same order. + +Two things contradicted it. + +**An archive handed back variables that were not interned.** Boost deserialization builds a +node through its default constructor, never through `Make`. `copy.deepcopy`, `copy.copy`, +`pickle`, and the MPI broadcast all load from an archive, so each returned a system +content-equal to the original and sharing none of its nodes. Such a system could not be +concatenated with the one it was copied from ("differing variable orderings"). The repair +existed -- `System::ReinternNodes`, used by the records loader -- and nothing else called it. + +**The operations that combine systems asked two different questions.** `Concatenate` +compared the variable orderings. `MakeHomotopy`, `MakeMovingHomotopy` and `+=` compared +counts: of variables, of homogenizing variables, of groups. The homotopy builders evaluate +their operands as whole systems, each fed the point by position, so they never needed the +variables to be shared -- and never noticed when they were not the same. Measured: moving +rows over `(y, x)` blended with a fixed system over `(x, y)` were accepted, and `y - 3` at +`(x, y) = (7, 11)` evaluated to 4. + +The two met in `straight_line_homotopy`, which calls the moving-row builder and then +concatenates to assemble `.target` and `.start`: it refused operands the builder underneath +it accepted. The cellular decomposition port, which deep-copies the three systems of its +projective move, found it. + +## Decision + +1. **Loading re-interns.** `System::serialize` and `Slice::serialize` re-intern what they + hold as the last step of a load. It is done in the load, not asked of each caller, so a + system that reaches anyone is over the live variables. Re-interning changes no content + (the digest is invariant) and drops the derived caches, which recompute on demand. + +2. **One check, on the variables.** `CheckVariableStructuresMatch` compares variable by + variable: the ordering, the affine and projective groups, the ungrouped variables, the + homogenizing variables. `Concatenate`, `MakeHomotopy`, `MakeMovingHomotopy` and `+=` all + make it. The refusal names both structures and says which kind of difference it is: a + different order, a different grouping, different variables. + +3. **A system that declares no variables takes the other's** (`AdoptVariableStructure`). + Functions with nothing said about variables are read as being over the variables of what + they are combined with. A system that declares any variables is compared, never adopted. + +4. **In Python a node compares by identity**: `__eq__`, `__ne__` and `__hash__` on the node + wrappers compare the node, not the wrapper. Each call across the binding makes a new + wrapper, and the default `==` said two handles on one node were different. + +## Consequences + +- A deep copy, a shallow copy and an unpickled system combine with their original, and with + each other. `clone` remains the direct way; it needs no archive. +- Systems over the same variables in a different order, or grouped differently, are refused + by the homotopy builders and `+=`, which used to accept them. That is a breaking change for + code that relied on matching by position, and such code was computing with its coordinates + exchanged. +- **Do not go back to counting** in `CheckVariableStructuresMatch`, and do not give an + operation its own comparison. Counts agree for `(x, y)` and `(y, x)`. +- **Do not remove the load hooks** on the grounds that a caller can re-intern: the callers + that did not are the reason for this record. A new class that holds nodes and can be + archived needs the same last step. +- **Do not build a node outside `Make`.** The protected default constructors exist for the + archive and for nothing else. +- Left alone on purpose: a function that mentions a variable no group holds. It is accepted + at construction, the degree questions read the stray variable as a constant, and + evaluation refuses ("a function references the variable 'y', which is not in the system's + variable ordering"). The rule for it -- every variable in a function has a declared role + in its system -- belongs with parameters, which are a role that does not exist yet. Users + may add functions and groups in any order, so that check cannot run at `AddFunction`. +- Pinned in `core/test/classes/system_identity_test.cpp` (the "combining systems" cases) and + `python/test/classes/clone_concatenate_test.py`. diff --git a/docs/adr/README.md b/docs/adr/README.md index fe3c8a533..3f8730d19 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -84,3 +84,4 @@ Each ADR follows the template: | [0065](0065-printing-is-presentation-and-lives-in-python.md) | Printing an object is PRESENTATION and lives in `bertini/_repr.py`, attached at import (a Boost.Python class accepts the assignment, and `str()` follows `repr()`), not in the bindings -- a C++ `operator<<` serves a different audience and can be written when one is wanted; machinery is described in angle brackets (``) because none of it could be rebuilt from text, while configs and metadata keep the call form; a tracker's description is LIVE, since the moment you print one is inside an observer callback; no repr may raise, because a throwing repr turns a debugging `print` into a traceback; a test fails if any public class inherits the default repr | Python bindings / printing | | [0066](0066-a-records-session-is-the-process.md) | A records session is the process: `OutputDirectory::Shared()` holds each instance for the life of the process (replacing it only for a deleted directory or a forked child), so a parameter sweep writes one history file instead of claiming one per solve and exhausting the 25 names a second allows; an idle instance holds no open files, and results files are capped at 16 open | Records | | [0067](0067-the-endgames-differ-in-root-estimation-only.md) | The endgames differ in root estimation ONLY: the security ceiling (two consecutive endpoint approximations above `max_norm`, at security level 0) is asked BEFORE acceptance in the Cauchy endgame as it is in the power series one, and an approximation above the ceiling is never accepted -- Cauchy had returned `Success` for finite roots above the ceiling that power series truncated; Cauchy's count still runs over operating-zone rounds only, since an out-of-zone mean is not an endpoint approximation (that condition keeps cyclic-6 at 156); don't move acceptance back above security, don't drop the `!above_ceiling` clause | Core / endgames | +| [0068](0068-combining-systems-compares-the-variables.md) | Combining systems compares THE VARIABLES, and a loaded system holds the live ones: `System` and `Slice` re-intern as the last step of loading from an archive (deep copies, pickles and the MPI broadcast came back over fresh variables of the same names and could not be recombined with their original); `Concatenate`, `MakeHomotopy`, `MakeMovingHomotopy` and `+=` make ONE check, variable by variable, group by group, in order -- the builders had counted, and blended `(x, y)` with `(y, x)` by position; a system that declares no variables takes the other's; in Python a node compares by identity, not by wrapper; don't go back to counting, don't remove the load hooks, don't build a node outside `Make` | Core / system, function tree; Python bindings | diff --git a/python/bertini/_system_ops.py b/python/bertini/_system_ops.py index aacbf7890..4ce284a9b 100644 --- a/python/bertini/_system_ops.py +++ b/python/bertini/_system_ops.py @@ -277,8 +277,9 @@ def clone(self): Shares the immutable node DAG (variables, functions, subexpressions) with the original but gets its own evaluation memory, so its variables line up with the original's (clone a set-up system, give the clone different functions, concatenate the two). The copy is unsealed and independently mutable; - adding/removing functions on one does not affect the other. For a fully serialized deep copy use - ``copy.deepcopy``. + adding/removing functions on one does not affect the other. ``copy.deepcopy`` and ``pickle`` give + a copy that combines with the original just as well; they go through an archive, which a clone + does not need to. """ from bertini.system import clone as _clone return _clone(self) diff --git a/python/docs/source/tutorials/formulating_and_solving/concatenate_systems/index.rst b/python/docs/source/tutorials/formulating_and_solving/concatenate_systems/index.rst index 1f541b527..e8a862c6f 100644 --- a/python/docs/source/tutorials/formulating_and_solving/concatenate_systems/index.rst +++ b/python/docs/source/tutorials/formulating_and_solving/concatenate_systems/index.rst @@ -124,10 +124,38 @@ block, concatenate, solve** -- scales to as many blocks as your problem comes in .. note:: - ``clone`` is the right tool here precisely because it shares the variables: the clone's - :math:`x_1` *is* the base's :math:`x_1`, so the blocks line up. If you instead need a fully - independent serialized copy (for example to send a system to another process), use - ``copy.deepcopy`` or :mod:`pickle`. + What makes the blocks line up is that they are over the same variables, and a variable is its + name: the clone's :math:`x_1` *is* the base's :math:`x_1`. The same holds for a system copied with + ``copy.deepcopy`` or sent through :mod:`pickle` (to another process, say), so those combine with + the original too. ``clone`` is the direct way, since it has no archive to go through. + +Two systems are combined only when they are over the same variables, in the same groups and the same +order. A point is a list of numbers, and the order of the variables is what says which number is +which, so systems over :math:`(x, y)` and :math:`(y, x)` are refused, not quietly matched up by +position: + +.. testcode:: + + u, v = bertini.Variable('u'), bertini.Variable('v') + + one_way = bertini.System() + one_way.add_variable_group(bertini.VariableGroup([u, v])) + one_way.add_function(u - 2) + + other_way = bertini.System() + other_way.add_variable_group(bertini.VariableGroup([v, u])) + other_way.add_function(v - 3) + + try: + bertini.system.concatenate(one_way, other_way) + except RuntimeError as refusal: + print('same variables in a different order' in str(refusal)) + +.. testoutput:: + + True + +A system that declares no variables at all takes the variables of the one it is combined with. Complete example ================ diff --git a/python/test/classes/clone_concatenate_test.py b/python/test/classes/clone_concatenate_test.py index c2be34ade..778b7cbec 100644 --- a/python/test/classes/clone_concatenate_test.py +++ b/python/test/classes/clone_concatenate_test.py @@ -242,6 +242,141 @@ def test_pickle_copy_deepcopy_preserve_eval_and_jacobian(roundtrip): _assert_close(_jac(c, sp, tm), _jac(s, sp, tm)) +@pytest.mark.parametrize("roundtrip", [ + lambda s: pickle.loads(pickle.dumps(s)), + copy.copy, + copy.deepcopy, + pb.system.clone, +]) +def test_a_copy_combines_with_its_original(roundtrip): + # Loading from an archive re-interns (ADR-0068), so a pickled, copied or deep-copied System is + # over THE SAME variables as its original and concatenates with it. It used to come back over + # fresh variables of the same names, and was refused: "differing variable orderings". + a, b = _two_systems() + u = pb.system.concatenate(a, roundtrip(b)) + assert u.num_functions() == 3 + + pt = np.array([complex(2.0, 0.3), complex(-1.0, 0.5)]) + _assert_close(_eval(u, pt), _eval(a, pt) + _eval(b, pt)) + assert [str(v) for v in u.variable_ordering()] == ['x', 'y'] + + +def test_separately_homogenized_deep_copies_build_a_homotopy(): + # The cellular port's projective move: three deep copies, homogenized one by one, the fixed one + # patched. straight_line_homotopy assembles .target and .start by concatenation, and refused. + x, y = pb.Variable('x'), pb.Variable('y') + + def over_xy(f): + s = pb.System() + s.add_variable_group(pb.VariableGroup([x, y])) + s.add_function(f) + return s + + fixed = copy.deepcopy(over_xy(x * x + y * y - 1)) + start = copy.deepcopy(over_xy(x - 2)) + end = copy.deepcopy(over_xy(y - 3)) + fixed.homogenize() + fixed.auto_patch() + start.homogenize() + end.homogenize() + + built = pb.nag_algorithm.straight_line_homotopy(end, start, fixed=fixed, gamma=1) + assert built.homotopy.num_functions() == built.target.num_functions() + assert [str(v) for v in built.target.variable_ordering()] == \ + [str(v) for v in fixed.variable_ordering()] + + +def _over(variables, function): + s = pb.System() + s.add_variable_group(pb.VariableGroup(list(variables))) + s.add_function(function) + return s + + +def test_a_different_order_is_refused_everywhere(): + # The homotopy builders used to count variables, and blended (x, y) with (y, x) by position: + # y - 3 at (x, y) = (7, 11) evaluated to 4. + x, y = pb.Variable('x'), pb.Variable('y') + xy = _over([x, y], x - 2) + yx = _over([y, x], y - 3) + fixed = _over([x, y], x + y - 5) + + for combine in (lambda: pb.system.concatenate(xy, yx), + lambda: pb.nag_algorithm.straight_line_homotopy(yx, xy, projectivize=False), + lambda: pb.nag_algorithm.straight_line_homotopy(yx, xy, fixed=fixed, + projectivize=False)): + with pytest.raises(RuntimeError, match='same variables in a different order'): + combine() + + +def test_the_same_order_blends_the_rows_it_was_given(): + x, y = pb.Variable('x'), pb.Variable('y') + fixed = _over([x, y], x + y - 5) + built = pb.nag_algorithm.straight_line_homotopy(_over([x, y], y - 3), _over([x, y], x - 2), + fixed=fixed, gamma=1, projectivize=False) + at_the_end = _eval(built.homotopy, np.array([complex(7.0), complex(11.0)]), complex(0.0)) + _assert_close(at_the_end, [complex(13.0), complex(8.0)]) # x + y - 5, and y - 3 + + +def test_a_different_grouping_and_different_variables_are_refused(): + x, y, z = pb.Variable('x'), pb.Variable('y'), pb.Variable('z') + one_group = _over([x, y, z], x - 1) + two_groups = pb.System() + two_groups.add_variable_group(pb.VariableGroup([x, y])) + two_groups.add_variable_group(pb.VariableGroup([z])) + two_groups.add_function(z - 1) + with pytest.raises(RuntimeError, match='the grouping does not'): + pb.system.concatenate(one_group, two_groups) + with pytest.raises(RuntimeError, match='different variables'): + pb.system.concatenate(_over([x, y], x - 1), _over([x, z], z - 1)) + + +def test_a_system_that_declares_no_variables_takes_the_others(): + x, y = pb.Variable('x'), pb.Variable('y') + declared = _over([x, y], x * x + y * y - 1) + bare = pb.System() + bare.add_function(x - y) + assert bare.num_variables() == 0 + + pt = np.array([complex(2.0), complex(5.0)]) + _assert_close(_eval(pb.system.concatenate(declared, bare), pt), [complex(28.0), complex(-3.0)]) + _assert_close(_eval(pb.system.concatenate(bare, declared), pt), [complex(-3.0), complex(28.0)]) + + +# ----------------------------- nodes compare by identity ----------------------------- + +def test_a_variable_made_twice_is_one_variable(): + # Variables are canonical by name, so both handles are on one node; Python's default == + # compared the two wrapper objects and said they differed. + assert pb.Variable('x') == pb.Variable('x') + assert not (pb.Variable('x') != pb.Variable('x')) + assert pb.Variable('x') != pb.Variable('y') + assert hash(pb.Variable('x')) == hash(pb.Variable('x')) + + +def test_equal_expressions_are_one_node(): + x, y = pb.Variable('x'), pb.Variable('y') + assert x * y + 1 == 1 + y * x + assert x * y != x + y + assert len({x, pb.Variable('x'), y}) == 2 + assert {x: 'first'}[pb.Variable('x')] == 'first' + + +def test_a_node_is_not_equal_to_something_that_is_not_a_node(): + x = pb.Variable('x') + assert not (x == 1) + assert x != 1 + assert not (x == 'x') + assert not (x == None) # noqa: E711 -- the comparison is the thing under test + + +def test_a_variable_group_compares_by_its_variables(): + x, y = pb.Variable('x'), pb.Variable('y') + assert pb.VariableGroup([x, y]) == [pb.Variable('x'), pb.Variable('y')] + assert pb.VariableGroup([x, y]) != [y, x] + assert _over([x, y], x - 1).variable_ordering() == [x, y] + + def test_pickle_roundtrip_is_independent_of_the_original(): # A pickled-and-restored System must be a genuine deep copy, not an alias. x, y = pb.Variable('x'), pb.Variable('y') diff --git a/python_bindings/src/node_export.cpp b/python_bindings/src/node_export.cpp index f427fb1ea..60bd909bb 100644 --- a/python_bindings/src/node_export.cpp +++ b/python_bindings/src/node_export.cpp @@ -73,6 +73,24 @@ namespace bertini{ static std::string NodeRepr(std::shared_ptr const& self) { return PrintPython(*self, true); } static std::string NodeClassic(std::shared_ptr const& self) { return PrintClassic(*self); } + // Equality is NODE IDENTITY. Nodes are interned, so two handles on equal expressions + // are handles on one node, and comparing the nodes is comparing the expressions: + // Variable('x') made twice is one x. Each call across the binding makes a new Python + // wrapper, though, and Python's default == compares the wrappers, which said two + // handles on the same node were different. Anything that is not a node is left to + // the other operand (NotImplemented), so `x == 1` is False rather than an error. + static boost::python::object NodeSameAs(std::shared_ptr const& self, boost::python::object const& other, bool want_equal) + { + boost::python::extract> as_node(other); + if (other.is_none() || !as_node.check()) + return boost::python::object(boost::python::handle<>(boost::python::borrowed(Py_NotImplemented))); + bool const same = self.get() == as_node().get(); + return boost::python::object(same == want_equal); + } + static boost::python::object NodeEq(std::shared_ptr const& self, boost::python::object const& other) { return NodeSameAs(self, other, true); } + static boost::python::object NodeNe(std::shared_ptr const& self, boost::python::object const& other) { return NodeSameAs(self, other, false); } + static long NodeHash(std::shared_ptr const& self) { return static_cast(std::hash{}(self.get())); } + template template void NodeVisitor::visit(PyClass& cl) const @@ -101,6 +119,13 @@ namespace bertini{ .def("to_classic", &NodeClassic, (arg("self")), "The Bertini 1 spelling: powers as ^, a complex constant as (re+im*I), every constant with all its digits. What the classic parser reads back, and what System.to_classic_input() writes for each function.") + .def("__eq__", &NodeEq, (arg("self"), arg("other")), + "True if `other` is the same node. Nodes are interned, so equal expressions are one node: Variable('x') == Variable('x'), and x*y + 1 == 1 + y*x. Comparing with something that is not a node is False.") + .def("__ne__", &NodeNe, (arg("self"), arg("other")), + "True unless `other` is the same node. See __eq__.") + .def("__hash__", &NodeHash, (arg("self")), + "A hash of the node's identity, consistent with ==, so nodes work as dictionary keys and in sets.") + .def("__add__",addNodeNode) .def("__add__",addNodeMpfr) .def("__radd__",&raddNodeMpfr) diff --git a/python_bindings/src/system_export.cpp b/python_bindings/src/system_export.cpp index ea13434f6..995202c0a 100644 --- a/python_bindings/src/system_export.cpp +++ b/python_bindings/src/system_export.cpp @@ -420,8 +420,8 @@ namespace bertini{ // post-deserialize fixups as Clone: rebuild the SLP's derivatives (the archived SLP's // derivative outputs do not round-trip faithfully) and normalize precision across the tree. // This makes copy.copy / copy.deepcopy work and lets Systems cross process boundaries - // (multiprocessing). As with clone, the result's variables are distinct node objects from - // the original's. + // (multiprocessing). Loading re-interns (System::serialize, ADR-0068), so the result's + // variables ARE the original's, and the copy can be combined with what it was copied from. struct SystemPickleSuite : boost::python::pickle_suite { static boost::python::tuple getinitargs(System const&) @@ -465,8 +465,8 @@ namespace bertini{ ; // free functions - def("concatenate", &Concatenate,(arg("self"), arg("other")), "concatenate two Systems to produce a new one. Appends the second's functions onto a copy of the first. The two must share variable ordering (cloning one from the other, or just reusing the same variables, guarantees this -- variables are canonical by name). If exactly one is patched, the result takes that patch."); - def("clone", &Clone,(arg("self")), "Copy a System. The copy shares the immutable node DAG (variables, functions, subexpressions) with the original but gets its own evaluation memory, so it is safe to evaluate concurrently AND its variables line up with the original's -- which is what lets you clone a set-up system, give the clone different functions, and concatenate the two (issue #256). Adding/removing functions on one does not affect the other. For a fully serialized deep copy use copy.deepcopy or pickle."); + def("concatenate", &Concatenate,(arg("self"), arg("other")), "concatenate two Systems to produce a new one. Appends the second's functions onto a copy of the first. The two must be over the same variables, in the same groups and the same order; variables are canonical by name, so systems built over the same names, clones, deep copies and unpickled systems all qualify. A system that declares no variables takes the other's. If exactly one is patched, the result takes that patch."); + def("clone", &Clone,(arg("self")), "Copy a System. The copy shares the immutable node DAG (variables, functions, subexpressions) with the original but gets its own evaluation memory, so it is safe to evaluate concurrently AND its variables line up with the original's -- which is what lets you clone a set-up system, give the clone different functions, and concatenate the two (issue #256). Adding/removing functions on one does not affect the other. copy.deepcopy and pickle give a copy that combines with the original just as well; they go through an archive, which a clone does not need to."); def("make_homotopy", &MakeHomotopy, (arg("target"), arg("start"), arg("path_variable")="t", arg("gamma")=std::shared_ptr()), "Form the gamma-trick straight-line homotopy H = (1-t)*target + gamma*t*start, with the path variable added. At t=1 the homotopy is gamma*start (so start's solutions are its roots) and at t=0 it is target. When start carries a structured block (e.g. a products-of-linears start system) the two systems are combined with a blend block; otherwise node arithmetic is used. gamma=None generates a random rational gamma. Pair with nag_algorithm.user_homotopy to solve.");