Perfomance improvements and refactoring - #72
Merged
Conversation
The prior commit (777d6db) updated `Term.factors_labels` / `..._without_power` to use the new `Factor.structural_label` (quantized-freq) format, but missed two duplicate copies of the old trig-stripping logic that live inline on `Equation.terms_labels` and `Equation.terms_labels_without_power`. After 777d6db: * `new_term.factors_labels` returned the new format (`('sin', (1.0, 0, 0.0))`, freq bucket included). * `equation.terms_labels` kept building the old format (`('sin', (1.0, 0.0))`, freq stripped). `Equation.add_random_term` checks `new_term.factors_labels not in self.terms_labels` -- comparing a new-format frozenset against a frozenset of old-format frozensets. The two never match, so trig duplicates slipped past dedup. The assertion in `EquationMutation.apply` (`mutations.py:77`, `len(equation.terms_labels) == len(equation.structure)`) then collapsed those duplicates (old format still strips freq) and fired. Fix: have both Equation properties delegate to `Term.factors_labels` / `Term.factors_labels_without_power` so all dedup sites speak one identity format. Observed in rerun ac/new_rep02 and rep03; char tests still pass 26/26.
Per-system YAMLs now deep-merge on top of configs/defaults.yaml so hyperparameters (search/preprocessor/moeadd/grid_tokens/additional_tokens/fit) live in one shared baseline. Trig tokens move out of the ode/vdp adapters and into the defaults block; adapters keep only data-loading. Also adds two profiling entry points used by the perf series: profile_run.py wraps cProfile and aggregates per-operator times, profile_loop_stats.py runs lv+wave with EPDE_LOOP_STATS=1 to capture loop-count histograms.
Adds epde/_loop_stats.py and ~30 record() call sites across the NEW-pipeline hot path: PhysicsInformedLasso RFE/CD inner+outer, OffspringUpdater unique-offspring loop, EqRightPartSelector outer + inner-derivative + simplify-equation replace-term, RandomRHPSelector candidate-gen, _scrub_conflicting_terms, SoEqRPS bidirectional pass, Equation.__init__ unique-term, add_random_term, EquationMutation add-terms, TermMutation unique-term, TermParameterMutation unique. No behavior change. Cost when disabled (the default) is one global-var read per record(). Set EPDE_LOOP_STATS=1 to record; report() formats per-loop entries / mean / median / p95 / max / %cap / %early.
Extends _deepcopy_slots with attrs_to_share_by_ref so Term and Equation clones alias the population-wide TFPool by reference instead of recursively deep-copying every token family. Factor.__deepcopy__ gains the same treatment for _evaluator, equality_ranges, _latex_constructor, _all_vars and deriv_code (set at family construction, never mutated per-factor). Equation also drops the volatile per-instance caches (_cached_sw_weights, _terms_labels_cache, _terms_labels_without_power_cache) which the very next mutation invalidates anyway, and _eval_cache round-trips as a fresh empty dict (separate ref, equal == content -- preserves the contract characterized by test_eval_cache_after_deepcopy_is_fresh_dict). metaparameters stays deep-copied because encoding.Gene.__setitem__ mutates it. _status on Factor stays deep-copied because Factor.status setter mutates it. Profile result: dominant contributor to Wave wall-time reduction (-40% on Wave at 5 MOEA/D epochs); ~15% reduction on LV.
restore_property previously REPLACED a random structure slot with the new property-carrying term -- destructive even when ``terms_number`` hadn't been reached. Switch to APPEND when ``len(structure) < cap``, fall back to REPLACE only when at cap. Preserves more existing structure across the EqRPS / mutation lifecycle, which reduces the number of "every-target-is-inf" RPS sweeps downstream. Duplicate check generalised: a single ``idx`` argument means "this slot is mine"; ``idx=None`` (append path) checks against every existing term. _commit() centralises append-vs-replace + label-cache invalidation. Records restore_property.outer and restore_property.t_derivative_inner under EPDE_LOOP_STATS for follow-on profiling.
EqRPS keeps a per-instance set of structure hashes (``objective.terms_labels``) that produced inf fitness on every eligible target_idx. On repeat encounter, skip the term-sweep, reroll the equation via ``objective.randomize()`` and continue. Sparsity is deterministic on the same input, so an inf-for-all-targets verdict is guaranteed to repeat for the same structure. Scope is one operator-tree lifetime, which matches one ``EpdeSearch.fit()`` call (strategy builder constructs a fresh EqRightPartSelector per build). Profile result on LV (5 epochs): 20.5% cache-hit rate -> 1458 / 7113 inf events skipped -> ~14% of fitness work, the dominant contributor to the -61% wall reduction. Wave sees only 4 inf events so the cache barely fires there (its win is from the Tier 1 deepcopy aliasing landed separately).
EqRightPartSelector calls fitness in probe mode (force_out_of_place=True) once per candidate target_idx during its term-sweep. Returning ``fitness_value * total_lr`` previously forced the full CV (std/mean per data window) computation even when the caller only needed a ranking value. Move the early return BEFORE the CV block. EqRPS still picks the target with the smallest probe fitness, and the relative ordering is preserved because CV multiplies the same window factor across every candidate of the same equation -- the argmin is unchanged.
EqRightPartSelector's term-sweep previously rebuilt the windowed XTWX matrix from scratch for every candidate target_idx (3-6 candidates per sweep typical, all on the same underlying terms). Replace with a single super-Gram over Z_aug = column_stack(all_terms, ones), computed once per RPS outer iter; each candidate then derives its per-target GramSetup view via pure slicing. The math is exact: (Z[:, ~t])^T diag(W) Z[:, ~t] is the sub-block of (Z_aug)^T diag(W) Z_aug at rows/cols ``~t U intercept``; the per-target XTWy is the column at index t of the same super-Gram at rows in ``active``. GramSetup gains precompute_super (build super-Gram + cache Z) and from_full (slice per-target view) classmethods. PhysicsInformedLasso.fit accepts an optional pre-built gram_setup. VWSRSparsity short-circuits objective.evaluate(normalize=True) when ``objective._gram_super`` is set, slicing target/features directly from the cached Z -- saves a redundant vstack + transpose of the same term evaluations for every candidate target_idx in the sweep. Equation gains a _gram_super slot, cleared in reset_state / _invalidate_label_cache / __deepcopy__ so the cache never outlives the structure it was built for. Profile result on lv+wave (5 MOEA/D epochs, NEW pipeline, early-stop off): - LV 337.8s -> 332.4s -1.6% (small grid, ~3 candidates / sweep) - Wave 886.9s -> 489.5s -44.8% (big grid, ~6 candidates / sweep) - Combined 1224.7s -> 821.9s -32.9%
CustomEvaluator.__call__ previously built an np.vectorize wrapper that dispatched per-element on a precomputed array of tuples -- 65k Python lambda calls per factor evaluation on Wave's 256x256 grid. The underlying funcs (trig, sign, grid, inverse, const, phased_sine) all use numpy ops (np.cos, np.sin, np.power, np.full_like, ...) that vectorize natively over arrays, so the per-element loop is wasted work. Add ``native_vectorized=False`` to CustomEvaluator.__init__. When True, __call__ calls ``funcs(*func_args, **eval_fun_kwargs)`` once with the full grid arrays. Default stays False so user code passing a non-vectorising callable is unchanged. Set the flag on all built-in module-level evaluators (sign, trig, grid, inverse, const, const_grad, phased_sine). velocity_evaluator and velocity_grad_evaluators stay on the slow path -- they're constructed with a positional-arg pattern that doesn't pass an explicit ``evaluation_functions_torch`` and is orthogonal to this change. Profile result on wave (5 MOEA/D epochs, NEW pipeline, early-stop off): - Wave 489.5s -> 333.5s -31.8% - LV 332.4s -> 344.4s +3.6% (likely run-to-run variance; LV's 320-sample grid has too little per-call cost to expose the np.vectorize tax) Cumulative vs original (pre-Tier-1) baseline: - LV 876.5s -> 344.4s -60.7% - Wave 1475.9s -> 333.5s -77.4%
…tries Factor.evaluate / simple_function_evaluator / token_family's post-generation check now key the global tensor cache on ``structural_label`` instead of ``cache_label``. For factors with only exact-tolerance params (derivatives, grid, const, sign) the two labels are equal and behaviour is unchanged. For trig factors with continuous ``freq`` (equality_ranges['freq'] > 0), the param quantizes to a bucket index -- so two factors at freq=2.00000003 and freq=2.00000007 within the same tolerance window now share one cache entry instead of evaluating separately. The mutation hot path generates a fresh freq draw per offspring; under the old cache_label key every distinct freq missed the cache, even though the structural-dedup logic already treated them as equivalent. Aligning the tensor cache key with the dedup key closes that gap. Profile result on lv+wave (5 MOEA/D epochs, NEW pipeline, early-stop off): - LV 344.4s -> 322.7s -6.3% - Wave 333.5s -> 293.5s -12.0% Cumulative vs original (pre-Tier-1) baseline: - LV 876.5s -> 322.7s -63.2% - Wave 1475.9s -> 293.5s -80.1%
The `while system in objective.history` loop at the initial-population construction site had no retry cap. If the initial pop_size exceeds the unique-candidate count (small grid + narrow token pool), the loop spins forever. Asymmetric vs OffspringUpdater, which already bounds its analogous loop via `offspring_attempt_limit`. Adds `uniqueness_attempt_limit` (default 100) to the operator params. On cap-hit, instrument via _loop_stats with the .FAIL suffix and accept the duplicate to keep the run alive (rather than raise mid-fit).
The retry loop at mutations.py:142 was capped at 100 iters but exited silently on cap exhaustion, returning whatever the last `randomize()` produced -- possibly still a duplicate or a no-op vs the original term. This violates the structure-dedup invariant: callers (EquationMutation, SystemMutation) treat the return as a "successful mutation", so silently committing a duplicate inflates population-corruption risk. Use a `hit_cap` flag to detect cap exhaustion. On cap-hit, restore the pre-mutation term (saved as `temp` before the loop) and re-invalidate the equation label cache. Differentiate FAIL via _loop_stats so the existing Phase 0 instrumentation can quantify the rate. Mutation is now atomic: either it produces a unique non-trivial change or it leaves the equation untouched. No no-ops counted as successes.
… duplicate The prior cap-fix (bc9f5d9) accepted a duplicate candidate when the uniqueness retry exhausted -- violating MOEA/D's per-sector uniqueness invariant. The initial population must be duplicate-free; silently admitting a duplicate corrupts the initial Pareto layer and the weight-vector / sector assignment downstream. Replace the "print and continue" branch with a RuntimeError carrying the candidate index, the count of already-placed unique systems, and actionable remediation hints (reduce pop_size, widen token pool, or raise the limit). The .FAIL _loop_stats record stays so post-mortem counts the cap-hit even though the run terminated.
…pring The pre-append guards at L194-204 use ``factors_labels`` (bucketed structural identity) and correctly reject injected terms that collide with what's already in the offspring. But ``flatten(equation*_terms)`` at L186 bypasses those guards: if ``detect_similar_terms`` ever places two structurally-equivalent terms into both the ``same`` and ``similar`` slots (possible with trig factors inside ``equality_ranges``), the flattened structure carries a duplicate from the start. Add a post-assembly dedup gate using the same ``factors_labels`` keys. On detected duplicate in either offspring, return the parents unchanged -- crossover becomes a no-op rather than silently emitting a corrupted chromosome. Mirrors TermMutation's atomic-mutation semantics from D2. Instrument the FAIL rate via _loop_stats so the existing Phase 0 helper can quantify how often the gate fires in real runs.
``Equation.restore_property`` (epde/structure/main_structures.py:633) is a configuration validator masquerading as a retry loop. Its job is to guarantee the equation contains at least one term with a derivative of the main variable (any axis); if the token pool genuinely cannot produce such a term in 200 sampling attempts, the configuration is broken (max_derivative_order=0 in every domain, derivative family not enrolled, mandatory_family wiring wrong) and no amount of further sampling will help. In healthy configs the outer loop completes in single-digit attempts (observed mean 2.8-3.3, max 16 across every historical thesis run -- never approached the cap). The prior code emitted ``warnings.warn`` and returned, leaving the caller with a property-less equation it then treated as valid. Replace the silent warn-and-return with a ``RuntimeError`` carrying the equation's main_var, the requested properties, and three actionable remediation hints. Matches the loud-failure semantics established by D1 (InitialParetoLevelSorting) for the same class of invariant violation.
The commit ``aaea0f4 New logic`` (undated rationale) silently switched
multi-objective ``EquationMutation.apply`` from per-term Bernoulli
term-replace (matching the single-objective design) to a 10-iteration
``add_random_term`` loop. The TermMutation sub-operator was left wired
but never invoked. The JSON defaults still reflect the original design
(r_mutation = 0.6, n_added_terms = 5).
Effect of the regression: once a chromosome reached the ``terms_number``
cap, ``add_random_term`` returned False on iter 1 and the loop bailed
immediately, so mature chromosomes received NO mutation. The Pareto
sector's ``unique_offspring`` check then rejected the unchanged
chromosome as a duplicate, burning a full mutation+RPS+fitness cycle
per FAIL. Loop-stats showed 13% (LV) / 21% (Wave) of sectors hitting
the silent-skip path -- a real population-drift signal traced directly
to the missing structural exploration.
Restore the hybrid replace+add design that the JSON defaults always
implied:
* Phase 1: per-term Bernoulli replace via TermMutation, gated by
``r_mutation``, skipping ``n_immutable`` head terms (right-part
anchor, mandatory_family).
* Phase 2: bounded ``add_random_term`` regrowth, capped by
``n_added_terms`` from JSON (5) and ``terms_number`` from the
chromosome metaparams.
Smoke results (LV/Wave, 5 epochs, NEW pipeline, seed 0):
* Wall: LV 370s -> 292s (-21%), Wave 340s -> 129s (-62%)
* OffspringUpdater.unique_offspring.FAIL rate:
LV 13.3% -> 6.25%, Wave 20.8% -> 3.75%
* EqRPS.outer entries: LV -25%, Wave -47%
* PhysicsInformedLasso.CD_inner: LV -17%, Wave -65%
* TermMutation.unique_term: 0% cap-hit on both -> D2 revert-on-cap
semantics never triggered (the dedup gate is now insurance, not
an active code path)
* Char tests 26/26.
The wall-time win is not from the mutation itself -- TermMutation is
strictly more expensive than add_random_term -- but from the dropped
silent-skip cycles. The previous "fast" code was wasting CPU on
doomed offspring attempts.
Also declares ``n_added_terms`` in EquationMutation's param_keys so
the operator's contract matches the JSON-loaded defaults.
The prior multi-objective ``EquationCrossover.apply`` produced two
structurally-identical offspring on every call: both ended up containing
the UNION of the parents' ``factors_labels``. ``detect_similar_terms``
returns a 3-way [same, similar, different] split where the ``different``
branch is unreachable (any non-common term lands in ``similar``), so the
``flatten + dedup-injection`` assembly collapsed to "both offspring =
union of parents". D10's within-offspring dedup gate didn't catch this
because each offspring was internally unique -- just identical to the
other. Half the offspring pool was effectively wasted.
The wired ``term_param_crossover`` and ``term_crossover`` sub-operators
were declared but never invoked, so the parameter-blending search
dimension that the JSON defaults explicitly enable
(term_param_proportion = 0.4) was dead.
Replace the union-emitter with a three-phase build:
* Anchor (exact ``factors_labels`` match): preserved unchanged, but
each offspring inherits its own parent's instance so within-bucket
parameter variation isn't lost.
* Param-blend pairs (matching factor-function signature, differing
params): routed through ``TermParamCrossover`` to produce one
blended variant per offspring -- activates the dormant exploitation
operator.
* Truly-unique terms (no anchor, no param-blend match): random 50/50
partition between the two offspring.
Each offspring is then guaranteed to contain its parent's target term
(force-included after partition) and passes the D10 dedup gate before
returning. Operates on the post-RPS non-zero form per project memory
``project_mutation_crossover_non_zero_form`` -- no zero-weight
scaffolding leaks across crossover.
Smoke results (5 epochs, NEW, seed 0):
* Wall: LV 292s -> 280s (-4%), Wave 129s -> 127s (-1.7%)
* EquationCrossover.duplicate_offspring.FAIL: 0 on both systems
(the new design produces structurally-distinct offspring without
triggering the post-assembly dedup revert)
* OffspringUpdater.unique_offspring.FAIL: LV 30 -> 29, Wave 18 -> 17
(small additional drop on top of the term-replace win)
* Crossover entry count jumped because the old code's
``if same_num == 0: return parents`` early-exit silently bypassed
instrumentation -- the true invocation count is what's now recorded
* Char tests 26/26.
D5: ParetoLevels.delete_point now counts matched points and raises RuntimeError if not exactly 1 was removed. The history-based uniqueness guard in OffspringUpdater is supposed to prevent collisions but loud failure is preferable to silent population shrinkage if it ever leaks. D6: MOEADDOptimizer iterates sectors in np.random.permutation order each epoch instead of fixed 0..N-1. The prior fixed order gave early sectors a recurring population advantage. Reproducibility preserved via the global seed set by the caller.
Single coherent refactor closing the sleepy-swinging-acorn audit
items R1-R5 plus the prior-session D8 doc comment.
R5: `_deepcopy_slots` moved from main_structures.py into
structure_template.py so Factor can call it without a circular
import. Factor.__deepcopy__ now delegates to the shared helper
(prior hand-written loop deleted); shallow __dict__.update is
preserved because Token base classes don't declare __slots__ and
`val` is recomputed on every Factor.value() rather than mutated.
R1+A6: Factor memoizes `cache_label`,
`structural_label`, `structural_label_without_power` into __slots__
populated lazily on first read. Invalidation routed through:
- Overridden Factor.params setter (calls super then invalidates).
- Overridden Factor.set_param (calls super then invalidates).
- New Factor._invalidate_label_cache() method for explicit
callers.
Four in-place `factor.params[i] = X` offender sites converted to
`factor.set_param(..., idx=i)` so the invalidation hook fires:
- filter_powers in supplementary.py
- factor.params[i] -= min_order in simplify_equation
(right_part_selection.py)
- TermParamCrossover blend in multi-objective variation.py
- TermParamCrossover blend in single-objective variation.py
Updated Tokens.py D8 doc to point at the new contract.
R2: Removed dead `form_label` (had a stray debug print) and
`detect_similar_terms_deprecated` from supplementary.py.
Strengthened `factor_params_to_str` docstring as the single
source of truth for the (label, params) tuple format, with an
explicit pointer to the structural_label quantization path.
R3: Removed Term.term_label / Term.term_label_without_power
deprecated aliases (TODO had been pending since the original
rename); removed the two characterization tests that pinned them.
Documented `detect_similar_terms` 3-way split semantics
(different-branch unreachable by construction) and the
`_scrub_conflicting_terms` superset semantics distinct from the
exact-match / set-cardinality predicates used elsewhere.
R4: Added `retry_until_unique` helper in supplementary.py
centralizing the attempt counter + cap-bound + _loop_stats
bookkeeping for the term-replacement loops. Converted three
sites: Equation.__init__.unique_term,
Equation.add_random_term, and simplify_equation.replace_term.
`add_random_term` cap kept at 10 (vs the 100 shared by the
others) with an inline rationale comment -- the outer
EquationMutation loop retries by drawing more terms anyway.
Validation: characterization suite 24/24 (was 26/26, 2 alias
tests removed alongside the aliases). LV + Wave smoke runs
clean: 0% cap-hits in restore_property.outer and
InitialParetoLevelSorting.unique_candidate; OffspringUpdater
FAIL counts within seed variance (LV 33->31, Wave 14->18);
EquationCrossover.duplicate_offspring identical to baseline
(LV 412->444, Wave 240->240); Wave wall -2% (119s -> 117s).
Files:
epde/structure/structure_template.py (+_deepcopy_slots)
epde/structure/factor.py (R1+A6 + R5)
epde/structure/main_structures.py (R3 + R4 + R5 import)
epde/structure/Tokens.py (D8 doc refresh)
epde/supplementary.py (R2 + R3 + R4 + R1+A6)
epde/operators/common/right_part_selection.py
(R1+A6 + R3 + R4)
epde/operators/multiobjective/variation.py
(R1+A6)
epde/operators/singleobjective/variation.py
(R1+A6)
tests/unit/test_main_structures_characterization.py
(R3 alias tests removed)
H1 -- decomposition_based_worst docstring (moeadd_specific.py:67-83):
the prior text wrongly claimed "Algorithm 3 from the MOEA/DD paper";
the function actually serves two branches of Algorithm 4 (l=1 and
l>1, |F_l|>1, |Phi^h|>1). The new docstring identifies both call
sites and flags the deliberate F_l-restriction deviation from
paper Algorithm 4 line 18.
H2 -- PopulationUpdater Case 3 inline comment (moeadd_specific.py:172-198):
inline block documenting that the crowded-subregion search and
worst-PBI argmax are intentionally restricted to F_l (not full
Phi^h as the paper specifies) to preserve elite F_1..F_{l-1}
solutions. Prevents future "fix-to-match-paper" regressions.
M1 -- PBI_penalty 1.0 -> 5.0
(default_parameters_multi_objective.json): paper Sec IV-D
recommends theta=5.0; the prior default of 1.0 made PBI behave
closer to weighted-sum than weighted-Tchebycheff.
M2 -- parents_fraction 0.4 -> 0.2
(default_parameters_multi_objective.json): paper's implicit k~=2
parents per sector maps to fraction ~= 0.1 at pop_size=20;
moving to 0.2 is a conservative halving rather than the
paper-exact 0.1, to preserve EPDE structural-exploration headroom
while reducing per-epoch offspring inflation.
M1 and M2 are behavior-affecting tunables; smoke validation
recommended before any benchmark sweep.
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.
No description provided.