Visualizer.visualize_combined: accept quick_update kwarg#1254
Merged
Conversation
``AnalysisFactor.visualize_combined`` forwards a ``quick_update``
keyword to the wrapped Analysis's ``Visualizer.visualize_combined``,
but the base signature on ``af.Visualizer.visualize_combined`` did not
declare it. Any user Visualizer that did not override the method (or
overrode it without ``**kwargs``) crashed mid-search with::
TypeError: Visualizer.visualize_combined() got an unexpected
keyword argument 'quick_update'
The crash only fired on graphical / joint-fit search paths whose
periodic-update cadence calls ``visualize_combined`` with
``quick_update=True`` partway through a long Dynesty chain. Short
runs that finished before the first periodic update silently dodged
it.
Add ``quick_update: bool = False`` to the base signature and document
that it is forwarded from the factor-graph dispatch so subclasses can
branch on quick-update vs full-update plotting if they want. Default
no-op behaviour is unchanged.
Adds three regression tests covering the signature, a direct call
with the kwarg, and an end-to-end ``AnalysisFactor.visualize_combined``
call path that previously crashed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
AnalysisFactor.visualize_combinedforwards aquick_updatekeyword to the wrapped Analysis'sVisualizer.visualize_combined, but the base signature onaf.Visualizer.visualize_combineddid not declare it. Any user Visualizer that did not override the method (or overrode it without**kwargs) crashed mid-search with:The crash only fires on graphical / joint-fit search paths whose periodic-update cadence calls
visualize_combinedwithquick_update=Truepartway through a long Dynesty chain. Short runs that finish before the first periodic update silently dodge the bug.Add
quick_update: bool = Falseto the base signature and document that it is forwarded from the factor-graph dispatch so subclasses can branch on quick-update vs full-update plotting if they want. Default no-op behaviour is unchanged.End-to-end discovered while running a real-data 20-cell-line GDSC2 graphical Hill-curve fit (
scripts/cancer/graphical.pyin the cosmology_and_cancer workspace), which crashed at the first periodic update. The fix unblocks workspaceVisualizeroverrides that follow the documented base signature.API Changes
Tiny additive signature change on a single method.
Visualizer.visualize_combinednow declaresquick_update: bool = Falseso callers may pass it (the factor-graph dispatch always does). Default no-op behaviour is preserved; subclasses that already override the method without**kwargsget unblocked. See full details below.Test Plan
pytest test_autofit/analysis/test_visualizer_combined_kwargs.py— 3 new testspytest test_autofit/non_linear test_autofit/graphical test_autofit/analysis -q— 442 tests pass (no regressions)scripts/cancer/graphical.py --sample=cancer_real__drug_1073 --total_datasets=20(a 123-D joint Dynesty fit) — previously crashed at the first periodic update, now runs end-to-end (~2 min wall-clock).Full API Changes (for automation & release notes)
Changed Signature
autofit.non_linear.analysis.visualize.Visualizer.visualize_combined(analyses, paths, instance, during_analysis, quick_update=False)— added trailing keyword argumentquick_updatewith defaultFalse. Matches the call site inautofit.graphical.declarative.factor.analysis.AnalysisFactor.visualize_combined. Default no-op body is unchanged.Migration
None required. Existing user
Visualizersubclasses that overridevisualize_combinedwithout**kwargsare now unblocked from a previously-silent crash on graphical / joint-fit search paths. Subclasses that want to branch on the quick-update path can opt in by accepting the new kwarg; otherwise the default value flows through.🤖 Generated with Claude Code