fix(engine): exact-species pattern methods are component-order-insensitive (issue #13)#14
Merged
Merged
Conversation
…h path — issue #13 The #9 session methods get_species_count / remove_species / set_species_count matched a live species only when the caller wrote its components in RuleMonkey's own canonical order: a swapped-order pattern (e.g. X(p~0,y) for the canonical X(y,p~0)) silently matched nothing and returned 0. add_species, by contrast, placed components by molecule-type declaration index (comp_type_index) and so already canonicalized — leaving set_species_count with a non-canonical pattern diffing against a wrong baseline of 0 and overshooting its target. pattern_to_complex_graph fed components to the canonical labeler in the pattern's *listed* order, while extract_complex (the live-pool side) feeds them in molecule-type declaration order, and canonical_label preserves the input's component-name slot layout — so the two produced different labels for the same physical species. Place pattern components by comp_type_index and remap bond endpoints to the new slots, matching extract_complex and NFsim's order-insensitive exact-species matcher. Regression test: tests/cpp/species_order_model.xml (X with stateful p~0~1 plus stateless y) drives get/add/set/remove through both component orders; without the fix it reports four mismatches and an over-removal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #13. The exact-species session methods from #9 —
get_species_count/remove_species/set_species_count— matched a live species only when the caller wrote its components in RuleMonkey's own canonical order. A semantically identical pattern with components swapped (e.g.X(p~0,y)for the canonicalX(y,p~0)) silently matched nothing and returned 0.add_species, by contrast, placed components by molecule-type declaration index (comp_type_index) viainstantiate_pattern_complex, so it already canonicalized. That asymmetry madeset_species_countwith a non-canonical pattern diff against a wrong baseline of 0 and overshoot its target — silently.Root cause
pattern_to_complex_graph(the parse-side bridge to the canonical labeler) fed each molecule's components tocanonical::ComplexGraphin the pattern's listed order, whereasextract_complex(the live-pool side) feeds them in molecule-type declaration order.canonical_labelpreserves the input's component-name slot layout (canonical.cpp §3.3 "component-order contract") and only reorders same-name components — so the two sides produced different labels for the same physical species, and the byte-equalspecies_countlookup missed.Fix
pattern_to_complex_graphnow places each component at the slot given by itscomp_type_index(declaration order) and remaps bond endpoints to the reordered slots — matchingextract_complexand NFsim's order-insensitive exact-species matcher. The fix is localized to that one function; the parser already resolvescomp_type_indexand verifies the pattern is fully specified, so the slot mapping is a bijection.Test
New
tests/cpp/species_order_model.xml— moleculeXwith a statefulp~0~1and a statelessy, seeded with 5000X(p~0,y)and a state-change ruleX(p~0) -> X(p~1).species_methods_testgainstest_component_order_insensitive, drivingget/add/set/removethrough both component orders. Without the engine fix it reports four mismatches and an over-removal exception; with it, all assertions pass.Full ctest suite (23 tests) green under the ASan preset, which cross-checks every canonical label against a from-scratch recompute.
Bumps version to 3.2.1 and adds a CHANGELOG
Fixedentry.