Skip to content

Fix FIRE integrator momentum, convergence, and backward-compatibility issues#27

Draft
Copilot wants to merge 3 commits into5-fix-issue-with-geometry-breaking-from-time-to-timefrom
copilot/sub-pr-6-again
Draft

Fix FIRE integrator momentum, convergence, and backward-compatibility issues#27
Copilot wants to merge 3 commits into5-fix-issue-with-geometry-breaking-from-time-to-timefrom
copilot/sub-pr-6-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

Addresses several correctness bugs in the FIRE minimizer and related infrastructure that caused geometry instability, broken momentum, and test failures when loading older .pkl model states.

Core FIRE algorithm fixes (integrators.py)

  • Velocity momentum destroyed every iteration: single_iteration_fire unconditionally reset geo._fire_velocity = np.zeros(...) after the hasattr guard, nullifying the guard's intent. Removed the reset; fire_minimization_loop now owns initialization via initialize_fire().
  • Write-back silently dropped: .flatten() returns a copy — writes to velocity.flatten()[dof] never persisted. Replaced with .reshape(-1) (view) so assignments write through to geo._fire_velocity.
  • Max iterations treated as convergence: check_if_fire_converged returned converged=True when iteration_count >= max_iter. Changed to False; stalling is not convergence.
  • 9-value unpack on 8-value return: newton_raphson returns 8 values; the remodelling call unpacked 9.

Non-convergence handling (vertexModel.py)

FIRE non-convergence fell through silently with just a logger.warning. Now calls iteration_did_not_converged(), which restores backup geometry and sets didNotConverge = True, matching the Euler branch behavior.

Backward compatibility (set.py)

Added __setstate__ using a _BACKWARD_COMPAT_DEFAULTS class constant so older pickled Set instances gain missing FIRE/integrator attributes on load without AttributeError.

Other correctness fixes

  • __init__.py: StreamHandler guard was matching FileHandler (a subclass) — now checks handler.stream in (sys.stdout, sys.stderr).
  • analyse_simulation.py: Removed unconditional integrator = 'euler' override that discarded the model's saved integrator.
  • find_required_purse_string.py: Sort dirs_within before selecting index 0 (deterministic); use hasattr() instead of falsy getattr to guard integrator/tolerance overrides.
  • geo.py: Added np.isfinite() guards for Vol, Area, and tri.Area in geometry validation; renamed loop variables cellc_cell, facec_face to eliminate module shadowing.
  • utils.py: y_axis_label check uses substring match instead of exact string to handle dynamic labels like "Purse string strength (t=6.0)".

Test infrastructure

  • tests.py: load_data uses absolute os.path.join(TEST_DIRECTORY, 'Tests_data', ...) path; returns consistent (None, None, v_model) tuple for .pkl files; normalizes OutputFolder for None, b'', and ''.
  • test_vertexModel.py: Fixed newtonRaphson.KgGlobalintegrators.KgGlobal; corrected fixture path; aligned "10 iterations" comments with actual 20 * dt0 window.
  • All 10 load_data('.pkl') callers updated to unpack _, _, vModel = load_data(...).

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI and others added 2 commits March 18, 2026 09:32
Co-authored-by: Pablo1990 <1974224+Pablo1990@users.noreply.github.com>
…aults, use hasattr() instead of getattr()

Co-authored-by: Pablo1990 <1974224+Pablo1990@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue with geometry breaking from time to time Fix FIRE integrator momentum, convergence, and backward-compatibility issues Mar 18, 2026
Copilot AI requested a review from Pablo1990 March 18, 2026 09:39
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.

2 participants