Skip to content

Gsm v2 - #111

Open
emuskardin wants to merge 59 commits into
masterfrom
gsm-v2
Open

Gsm v2#111
emuskardin wants to merge 59 commits into
masterfrom
gsm-v2

Conversation

@emuskardin

@emuskardin emuskardin commented Sep 9, 2026

Copy link
Copy Markdown
Member

Blocking

1. has_local_compatibility / has_score_function are inverted — aalpy/learning_algs/general_passive/ScoreFunctionsGSM.py:91
Both return True when the method is not overridden. Combined with the guard at line 170, SimpleFutureBasedCompatibility(local_compatibility=..., ...) always raises.
Repro: run_GSM(data, transition_behavior="stochastic")ValueError("Exernal local compatibility is provided…") at GeneralizedStateMerging.py:130. Also breaks Examples.example_Alergia_extension and gsm_IOAlergia_domain_knowldege.

2. CountDataHandler.copy drops the defaultdictaalpy/learning_algs/general_passive/DataHandler.py:264
copy rebuilds transition_count as a plain dict, but merge (line 257) relies on defaultdict (x.transition_count[in_sym]). Any reversible (first-pass) partitioning over count data crashes.
Repro: run_GSM(..., transition_behavior='stochastic', score_calc=SimpleScoreCalculation(score_function=...))KeyError: 'b'.

3. add_labeled_sequence never calls init_data()aalpy/learning_algs/general_passive/DataHandler.py:104
Children are created as GsmNode((in_sym, out_sym), curr_node) without self.init_data(), unlike add_trace at line 76, so data is None.
Repro: CountDataHandler().createPTA(labeled_sequences)AttributeError: 'NoneType' object has no attribute 'transition_count'.

4. Deterministic compatibility is no longer enforced — aalpy/learning_algs/general_passive/GeneralizedStateMerging.py:385
The removed compute_local_compatibility enforced GsmNode.deterministic_compatible for transition_behavior="deterministic" regardless of score_calc; the replacement only checks Moore compatibility, and only when output_behavior == "moore". The same hunk downgrades the nondeterministic-input check to warnings.warn (line 179), removing the second guard.
Repro: a mealy/deterministic run with a user-supplied score_calc (Examples.gsm_edsm, or any SimpleScoreCalculation(score_function=...)) yields a root with 'a': ['x','y'] and is_deterministic() == False, which to_automaton then silently collapses.

5. CacheSUL.__getattr__ recurses infinitely — aalpy/base/SUL.py:136
Recurses whenever self.sul is absent, e.g. the __setstate__ lookup during unpickling.
Repro: pickle.loads(pickle.dumps(CacheSUL(AutomatonSUL(dfa))))RecursionError.
Fix: guard with if name == 'sul': raise AttributeError(name) (or use object.__getattribute__).

Correctness

6. best_candidate update is nested in the cache-miss branch — GeneralizedStateMerging.py:216
The best_candidate / best_score update sits inside if partitioning is None:. Cached partitionings survive promotions (only merges clear the cache), so from the round after a promotion onward a cached candidate can never be selected as best — even one with ImmediateAccept. The best is then chosen only among freshly computed pairs, giving a different/worse merge sequence than intended.

7. ScoreCombinator fakes an early verdict — ScoreFunctionsGSM.py:329
initialize_merge aggregates sub-results with aggregate_score, whose default returns a list. Sub-scores with no early verdict return None, so the result is [None, None], which is not SpecialScores.NoScore. GSM treats that as an early verdict, never builds the partitioning, and never calls the combinator's local_compatibility or score_function — every ScoreCombinator accepts merges on a meaningless score.

8. CountOnPTADataHandler.copy loses PTA state — DataHandler.py:275
It inherits copy from CountDataHandler, which returns a bare CountData, dropping pta_count and shadow_pta and changing the type. When such a copy is written back (GeneralizedStateMerging.py:250), later hoeffding_compatibility(compare_original=True) raises AttributeError: pta_count, and SimpleFutureBasedCompatibility raises "no PTA data is available". Reachable whenever CountOnPTADataHandler is combined with a score calc that has no early verdict.

9. EDSM_score checks are dead — GsmAlgorithms.py:40
It still tests node.get_prefix_output() is None to skip "no evidence" nodes, but unknown_output changed from None to object() (GsmNode.py:32). Both checks (lines 40 and 44) are now dead, so unlabeled nodes count as evidence and run_EDSM's scoring differs from its intent.

10. Stale cached partitioning misses newly-blue successors — GeneralizedStateMerging.py:241
After a promotion only the promoted blue's cache entries are dropped, but red_states_backing_set has grown. Cached partitionings captured red_partitions at first-pass time; when such a candidate is later applied the second pass returns early and reuses the stale new_blue list, so successors that should have become blue are never enqueued. Currently masked by finding 6 — it goes live once that is fixed.

11. moore_check uses red instead of partitionGeneralizedStateMerging.py:386
It calls GsmNode.moore_compatible(red, blue) on the original red rather than on partition, whose prefix output may already have been resolved by resolve_unknown_prefix_output. The check is strictly more permissive than intended and can let a Moore-incompatible implied merge through.

API / reporting

12. run_Alergia_EDSM dropped from package exports — aalpy/learning_algs/__init__.py:15
Replaced by run_Alergia_GSM, so from aalpy.learning_algs import run_Alergia_EDSM — previously public API — now fails. The alias survives only inside GsmAlgorithms (line 160), which is what the test imports.

13. nr_merged_states is double-counted — GeneralizedStateMerging.py:381
Incremented in both passes, so pairs deferred to remaining_merges are counted twice. ProgressReport.log_merge (Instrumentation.py:98) accumulates this, making the printed "Remaining" count drift and go negative.

14. First broken promotion goes unreported — Instrumentation.py:154
In MergeViolationDebugger.log_promote the new elif node is None branch is unreachable for the first broken promotion: self.map.get(None) is None, so the old_red is None branch fires first, inserts None as a map key and logs a normal "promote". Only the second broken promotion is reported.

zwergziege and others added 30 commits September 29, 2025 17:58
…oreCalculation.__init__`. also some other score calc stuff and docstrings
@emuskardin
emuskardin requested review from zwergziege and removed request for zwergziege September 9, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants