diff --git a/.gitignore b/.gitignore index 7a092c7..a9041a3 100644 --- a/.gitignore +++ b/.gitignore @@ -111,3 +111,6 @@ pyjibe/_version.py # uv uv.lock + +# ai +CLAUDE.md diff --git a/CHANGELOG b/CHANGELOG index e0a9451..030e785 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.16.4 + - fix: correctly handle params states when swapping to new model (#51) + - fix: ensure identical results when clicking per file and compute button (#47, #51) - enh: show chi^2 annotation and toggle fit line visibility (#48, #52) - setup: support latest python versions (#32, #46) - setup: support latest matplotlib versions (#32, #46) diff --git a/pyjibe/fd/main.py b/pyjibe/fd/main.py index df1735c..a98b0fb 100644 --- a/pyjibe/fd/main.py +++ b/pyjibe/fd/main.py @@ -513,6 +513,8 @@ def on_fit_all(self): try: # preprocessing could fail for bad data self.tab_preprocess.apply_preprocessing(fdist) + # update model key and ancillary parameters before fitting + self.tab_fit.fit_update_parameters(fdist) # external fitting model could fail self.tab_fit.fit_approach_retract(fdist, update_ui=False) except BaseException as e: @@ -522,6 +524,10 @@ def on_fit_all(self): try: # updating curve list may not be possible self.curve_list_update(item=ii) + # curve_list_update computes the rating; autosave + # again so the exported TSV reflects it (mirrors + # on_curve_list, which does the same) + self.autosave(fdist) except BaseException as e: logger.error(traceback.format_exc()) errored.append([fdist.path, e.__class__.__name__, e.args]) diff --git a/pyjibe/fd/tab_fit.py b/pyjibe/fd/tab_fit.py index 65d8464..2f18f7b 100644 --- a/pyjibe/fd/tab_fit.py +++ b/pyjibe/fd/tab_fit.py @@ -115,10 +115,14 @@ def anc_update_parameters(self, fdist): if (atab.item(row, 0).checkState() == QtCore.Qt.CheckState.Checked): # update initial parameters + # (block signals to prevent recursive on_params_init + # while applying ancillary values to itab) + itab.blockSignals(True) for rr in range(itab.rowCount()): if itab.verticalHeaderItem(rr).text() == label: if value_text != "nan": itab.item(rr, 1).setText(value_text) + itab.blockSignals(False) row += 1 atab.blockSignals(False) else: @@ -345,14 +349,21 @@ def fit_update_parameters(self, fdist): # set the model # - resets params_initial if model changed # - important for computing ancillary parameters + prev_model_key = fdist.fit_properties.get("model_key") fdist.fit_properties["model_key"] = model_key - if fdist.fit_properties.get("params_initial", False): - # (cannot coerce this into one line, because "params_initial" - # can be None.) + if prev_model_key != model_key: + # Clear the ancillary cache so anc_update_parameters gets fresh + # ancillaries for the new model (the cache may contain stale + # values from the previous model's fit) + fdist._anc_cache = None + if (fdist.fit_properties.get("params_initial", False) + and prev_model_key == model_key): # set the parameters of the previous fit params = fdist.fit_properties["params_initial"] else: - # use the initial model parameters + # use the initial model parameters (also when model changed, to + # ensure itab is set up with the new model's parameter labels so + # that anc_update_parameters can correctly apply ancillary values) params = self.fit_parameters() # parameter table diff --git a/tests/test_fd_fit.py b/tests/test_fd_fit.py index f5e9ac9..7c359fd 100644 --- a/tests/test_fd_fit.py +++ b/tests/test_fd_fit.py @@ -12,21 +12,19 @@ from helpers import MockModelModule, make_directory_with_data - data_path = pathlib.Path(__file__).parent / "data" def test_ancillary_update_init(qtbot): with MockModelModule( - compute_ancillaries=lambda x: { - # take initial fit parameter of E - "E": x.get_initial_fit_parameters( - model_ancillaries=False)["E"].value}, - parameter_anc_keys=["E"], - parameter_anc_names=["ancillary E guess"], - parameter_anc_units=["Pa"], + compute_ancillaries=lambda x: { + # take initial fit parameter of E + "E": x.get_initial_fit_parameters( + model_ancillaries=False)["E"].value}, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], model_key="test1"): - main_window = pyjibe.head.PyJibe() qtbot.addWidget(main_window) main_window.load_data(files=make_directory_with_data(2)) @@ -63,12 +61,11 @@ def test_ancillary_update_init(qtbot): def test_ancillary_update_nan(qtbot): with MockModelModule( - compute_ancillaries=lambda x: {"E": np.nan}, - parameter_anc_keys=["E"], - parameter_anc_names=["ancillary E guess"], - parameter_anc_units=["Pa"], + compute_ancillaries=lambda x: {"E": np.nan}, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], model_key="test1"): - main_window = pyjibe.head.PyJibe() qtbot.addWidget(main_window) main_window.load_data(files=make_directory_with_data(2)) @@ -93,12 +90,12 @@ def test_ancillary_update_nan(qtbot): def test_ancillary_update_preproc_change(qtbot): with MockModelModule( - compute_ancillaries=lambda x: { - # i.e. model works only if there are multiple preproc steps - "E": np.nan if len(x.preprocessing) == 1 else 2345}, - parameter_anc_keys=["E"], - parameter_anc_names=["ancillary E guess"], - parameter_anc_units=["Pa"], + compute_ancillaries=lambda x: { + # i.e. model works only if there are multiple preproc steps + "E": np.nan if len(x.preprocessing) == 1 else 2345}, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], model_key="test1"): main_window = pyjibe.head.PyJibe() @@ -138,6 +135,43 @@ def test_ancillary_update_preproc_change(qtbot): main_window.close() +def test_anc_update_does_not_recurse_via_itab_signal(qtbot): + """Writing an ancillary value into itab must not re-trigger itself.""" + calls = [] + + def compute_anc(x): + calls.append(1) + return {"E": 4242} + + with MockModelModule( + compute_ancillaries=compute_anc, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], + model_key="test_recursion"): + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=make_directory_with_data(1)) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(0) + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + idx = war.tab_fit.cb_model.findData("test_recursion") + war.tab_fit.cb_model.setCurrentIndex(idx) + + fdist = war.data_set[0] + itab = war.tab_fit.table_parameters_initial + itab.blockSignals(True) + itab.item(0, 1).setText("0") # real change, not a no-op + itab.blockSignals(False) + + calls.clear() + war.tab_fit.anc_update_parameters(fdist) + assert len(calls) == 1, ( + f"anc_update_parameters recursed via itab's itemChanged " + f"signal ({len(calls)} calls instead of 1)") + main_window.close() + + @pytest.mark.filterwarnings('ignore::UserWarning') def test_apply_and_fit_all_with_bad_data(qtbot, monkeypatch): # setup data directory with two good and one invalid file @@ -206,6 +240,106 @@ def test_change_model_keep_parms(qtbot): main_window.close() +def test_fit_all_matches_single_fit(qtbot): + """on_fit_all must produce the same TSV output as fitting one-by-one.""" + files = make_directory_with_data(2) + tsv_path = files[0].parent / "pyjibe_fit_results_leaf.tsv" + + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=files) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(True) + war._autosave_override = 1 # always overwrite, avoid dialog + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + war.tab_fit.cb_weight_cp.setCheckState(QtCore.Qt.CheckState.Unchecked) + + # Click through both files via the single-curve path + cl1 = war.list_curves.currentItem() + cl2 = war.list_curves.itemBelow(cl1) + war.list_curves.setCurrentItem(cl1) + war.list_curves.setCurrentItem(cl2) + assert tsv_path.exists(), "autosave TSV not created after single-fit" + single_tsv = tsv_path.read_text() + + # Re-fit everything via fit-all; TSV must be byte-for-byte identical + war.on_fit_all() + fitall_tsv = tsv_path.read_text() + + assert single_tsv == fitall_tsv + main_window.close() + + +def test_fit_all_matches_single_fit_kvm(qtbot): + """fit-all must produce the same TSV as single-fit for the KVM model. + + KVM has ancillary parameters (eta, time_ind) that are derived by fitting + a preliminary Hertz model to each curve. The KVM model is loaded via + PyJibe's extension system at startup. + """ + files = make_directory_with_data(2) + tsv_path = files[0].parent / "pyjibe_fit_results_leaf.tsv" + + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=files) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(True) + war._autosave_override = 1 # always overwrite, avoid dialog + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + war.tab_fit.cb_weight_cp.setCheckState(QtCore.Qt.CheckState.Unchecked) + + # Switch to the KVM model (loaded via PyJibe's extension system) + idx = war.tab_fit.cb_model.findData("hertz_corr_visco_KVM") + if idx < 0: + pytest.skip("KVM model extension not installed in this environment") + war.tab_fit.cb_model.setCurrentIndex(idx) + + # Click through both files via the single-curve path + cl1 = war.list_curves.currentItem() + cl2 = war.list_curves.itemBelow(cl1) + war.list_curves.setCurrentItem(cl1) + war.list_curves.setCurrentItem(cl2) + assert tsv_path.exists(), "autosave TSV not created after single-fit" + single_tsv = tsv_path.read_text() + + # Re-fit everything via fit-all; TSV must be byte-for-byte identical + war.on_fit_all() + fitall_tsv = tsv_path.read_text() + + assert single_tsv == fitall_tsv + + +def test_fit_all_rates_last_curve_before_final_autosave(qtbot): + """on_fit_all must not leave the last curve's rating as nan""" + files = make_directory_with_data(2) + tsv_path = files[0].parent / "pyjibe_fit_results_leaf.tsv" + + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=files) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(True) + war._autosave_override = 1 + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + war.tab_fit.cb_weight_cp.setCheckState(QtCore.Qt.CheckState.Unchecked) + + # call fit-all directly on freshly-loaded curves - no prior + # single-click pass + war.on_fit_all() + + rows = tsv_path.read_text().splitlines() + header = rows[0].split("\t") + regressor_col = header.index("Regressor") + rating_col = header.index("Rating") + assert len(rows) == 3 # header + 2 curves + for row in rows[1:]: + cells = row.split("\t") + assert cells[regressor_col] != "nan" + assert cells[rating_col] != "nan" + main_window.close() + + def test_remember_initial_params(qtbot): main_window = pyjibe.head.PyJibe() qtbot.addWidget(main_window) @@ -257,7 +391,7 @@ def test_set_indentation_depth_manually_infdoublespinbox(qtbot): ["1.10201", 1.10201], ["1.001", 1.001], ["-1.04", -1.04] - ]: + ]: war.tab_fit.sp_range_1.clear() qtbot.keyClicks(war.tab_fit.sp_range_1, text_entered) assert war.tab_fit.sp_range_1.value() == resulting_value @@ -291,7 +425,7 @@ def test_show_fit_line_toggle(qtbot): main_window.close() -def test_chi_sqr_annotation_shown_after_fit(qtbot): +def test_show_chi_sqr_annotation_after_fit(qtbot): """Chi^2 annotation should be visible after a successful fit.""" main_window = pyjibe.head.PyJibe() qtbot.addWidget(main_window) @@ -309,3 +443,152 @@ def test_chi_sqr_annotation_shown_after_fit(qtbot): chi2_value = float(text.split("=")[1].strip()) assert 0.0 <= chi2_value <= 1.0 main_window.close() + + +def test_swap_model_does_not_raise_and_updates_labels(qtbot): + """Model swap must not crash and itab must show new model params. + + Notes + ----- + In nanite>=4.2.3, FitProperties.__setitem__ already resets + "params_initial" whenever "model_key" changes (see nanite/fit.py), + so the stale-params KeyError from 8140997 is not reproducible via + normal UI actions here. This guards against a crash/label + regression during model swap instead. + """ + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=make_directory_with_data(1)) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(0) + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + + fdist = war.data_set[0] + assert fdist.fit_properties["model_key"] == "hertz_para" + assert "R" in fdist.fit_properties["params_initial"] + + pyr_name = nmodel.model_hertz_three_sided_pyramid.model_name + pyr_idx = war.tab_fit.cb_model.findText(pyr_name) + war.tab_fit.cb_model.setCurrentIndex(pyr_idx) # must not raise + + itab = war.tab_fit.table_parameters_initial + labels = [itab.verticalHeaderItem(rr).text() + for rr in range(itab.rowCount())] + assert any("Face Angle" in lb for lb in labels) + assert fdist.fit_properties["model_key"] == "hertz_pyr3s" + main_window.close() + + +def test_swap_model_recomputes_ancillary_not_cached(qtbot): + """Ancillary cache must be cleared (not reused) across a model swap.""" + with MockModelModule( + compute_ancillaries=lambda x: {"E": 1000}, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], + model_key="test_anc_a"): + with MockModelModule( + compute_ancillaries=lambda x: {"E": 9999}, + parameter_anc_keys=["E"], + parameter_anc_names=["ancillary E guess"], + parameter_anc_units=["Pa"], + model_key="test_anc_b"): + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=make_directory_with_data(1)) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(0) + war.tab_preprocess.set_preprocessing( + ["compute_tip_position"]) + war.tab_fit.cb_weight_cp.setCheckState( + QtCore.Qt.CheckState.Unchecked) + + idx_a = war.tab_fit.cb_model.findData("test_anc_a") + war.tab_fit.cb_model.setCurrentIndex(idx_a) + fdist = war.data_set[0] + + # real successful fit under model A populates _anc_cache + war.tab_fit.fit_approach_retract(fdist) + assert fdist.fit_properties.get("success") + war.tab_fit.anc_update_parameters(fdist) + assert fdist._anc_cache # sanity: cache now populated + + idx_b = war.tab_fit.cb_model.findData("test_anc_b") + war.tab_fit.cb_model.setCurrentIndex(idx_b) # must recompute + + atab = war.tab_fit.table_parameters_anc + assert atab.item(0, 1).text() == "9999" # fresh, not "1000" + main_window.close() + + +def test_swap_to_kvm_output_matches_unswapped_fit(qtbot): + """A confirmed fit under the default model, followed by a swap to + KVM, must not change KVM's ancillary-seeded initial defaults or + its fit output compared to swapping without that extra prior fit. + + KVM's "eta"/"time_ind" are fixed (non-varying) parameters seeded + from a real auxiliary fit (see compute_ancillaries in the KVM + extension) - unlike hertz_para/hertz_pyr3s used elsewhere in this + file, a stale ancillary cache here does not just cosmetically + mislabel the UI: it silently falls back to KVM's own hard-coded + literal defaults (eta=0.5, time_ind=1), which are baked into the + fitted model equation and change the final numeric fit output. + """ + tsv_name = "pyjibe_fit_results_leaf.tsv" + + def setup_window(files): + main_window = pyjibe.head.PyJibe() + qtbot.addWidget(main_window) + main_window.load_data(files=files) + war = main_window.subwindows[0].widget() + war.cb_autosave.setChecked(True) + war._autosave_override = 1 + war.tab_preprocess.set_preprocessing(["compute_tip_position"]) + war.tab_fit.cb_weight_cp.setCheckState( + QtCore.Qt.CheckState.Unchecked) + return main_window, war + + def swap_to_kvm_and_export(war, files): + idx = war.tab_fit.cb_model.findData("hertz_corr_visco_KVM") + if idx < 0: + pytest.skip( + "KVM model extension not installed in this environment") + war.tab_fit.cb_model.setCurrentIndex(idx) + fdist = war.data_set[0] + assert fdist.fit_properties.get("success") + kvm_defaults = war.tab_fit.fit_model.get_parameter_defaults() + + # eta/time_ind must be the freshly computed ancillary guesses, + # not KVM's own hard-coded literal defaults (which is what a + # stale/mismatched ancillary cache would silently fall back to) + itab = war.tab_fit.table_parameters_initial + eta_row = next( + rr for rr in range(itab.rowCount()) + if itab.verticalHeaderItem(rr).text().startswith("viscosity")) + time_row = next( + rr for rr in range(itab.rowCount()) + if itab.verticalHeaderItem(rr).text().startswith( + "Time to indent")) + assert (float(itab.item(eta_row, 1).text()) + != kvm_defaults["eta"].value) + assert (float(itab.item(time_row, 1).text()) + != kvm_defaults["time_ind"].value) + + return (files[0].parent / tsv_name).read_text() + + # path A: fit under the default model first, then swap to KVM + files_a = make_directory_with_data(1) + main_window_a, war_a = setup_window(files_a) + fdist_a = war_a.data_set[0] + war_a.tab_fit.fit_approach_retract(fdist_a) # confirmed hertz_para fit + assert fdist_a.fit_properties.get("success") + tsv_a = swap_to_kvm_and_export(war_a, files_a) + main_window_a.close() + + # path B: identical, but without the extra prior fit + files_b = make_directory_with_data(1) + main_window_b, war_b = setup_window(files_b) + tsv_b = swap_to_kvm_and_export(war_b, files_b) + main_window_b.close() + + assert tsv_a == tsv_b