Skip to content

Ensure fitresult.covar is not None before processing#104

Closed
martinvonk wants to merge 1 commit into
timflow-org:devfrom
martinvonk:patch-1
Closed

Ensure fitresult.covar is not None before processing#104
martinvonk wants to merge 1 commit into
timflow-org:devfrom
martinvonk:patch-1

Conversation

@martinvonk
Copy link
Copy Markdown

I'm running into an issue where the fitresult.covar is None.

Fit succeeded. Could not estimate error-bars.
[[Fit Statistics]]
    # fitting method   = leastsq
    # function evals   = 34
    # data points      = 300
    # variables        = 3
    chi-square         = 22518.4864
    reduced chi-square = 75.8198197
    Akaike info crit   = 1301.49282
    Bayesian info crit = 1312.60417
##  Warning: uncertainties could not be estimated:
    c_0_0:    at initial value
[[Variables]]
    kaq_0_1:  256.584548 (init = 100)
    c_0_0:    1000.00000 (init = 1000)
    Saq_0_1:  7.4078e-05 (init = 0.0001)

Add check for non-null covariance matrix in fit result
@mbakker7
Copy link
Copy Markdown
Contributor

Thanks for reporting this. What happens right now when the covar cannot be computed? Does it bomb or does it simply not compute stderr.
We are in the process of updating this internally. In fact, the current plan is to not compute stderr at all, as it rarely is a reasonable estimate since the residuals are highly correlated.

@martinvonk
Copy link
Copy Markdown
Author

martinvonk commented May 11, 2026

It now just crashes on np.sqrt(np.diag(self.fitresult.covar)) because self.fitresult.covar is None. So I think it would be nice to at least solve this to prevent crashes. You can decide on showing the stderr or not for cases that don't crash :)

See below:

Cell In[3], [line 53](vscode-notebook-cell:?execution_count=3&line=53)
     52 cal.series(name="obs", x=x, y=y, t=t_obs, h=h_obs, layer=0)
---> [53](vscode-notebook-cell:?execution_count=3&line=53) cal.fit(report=False)

File ~.venv/lib/python3.13/site-packages/timflow/transient/fit.py:391, in Calibrate.fit(self, report, printdot, **kwargs)
    388 def fit(self, report=False, printdot=True, **kwargs):
    389     # current default fitting routine is lmfit
    390     # return self.fit_least_squares(report) # does not support bounds by default
--> [391](.venv/lib/python3.13/site-packages/timflow/transient/fit.py:391)     return self.fit_lmfit(report, printdot, **kwargs)

File ~.venv/lib/python3.13/site-packages/timflow/transient/fit.py:348, in Calibrate.fit_lmfit(self, report, printdot, **kwargs)
    344     self.parameters.loc[name, "optimal"] = self.fitresult.params.valuesdict()[
    345         name
    346     ]
    347 if hasattr(self.fitresult, "covar"):
--> [348](.venv/lib/python3.13/site-packages/timflow/transient/fit.py:348)     self.parameters["std"] = np.sqrt(np.diag(self.fitresult.covar))
    349     self.parameters["perc_std"] = (
    350         100 * self.parameters["std"] / np.abs(self.parameters["optimal"])
    351     )
    352 else:

File ~/.venv/lib/python3.13/site-packages/numpy/lib/_twodim_base_impl.py:328, in diag(v, k)
    326     return diagonal(v, k)
    327 else:
--> [328](~.venv/lib/python3.13/site-packages/numpy/lib/_twodim_base_impl.py:328)     raise ValueError("Input must be 1- or 2-d.")

ValueError: Input must be 1- or 2-d.

@mbakker7
Copy link
Copy Markdown
Contributor

Well that is no good. So your fix is probably needed until we move forward on the stderr issue.

@mbakker7 mbakker7 changed the base branch from main to dev May 11, 2026 08:10
@dbrakenhoff
Copy link
Copy Markdown
Contributor

I removed the parameter uncertainty stuff in the unified calibration class. We could consider just removing the computation of that stuff here as well? Otherwise the proposed fix seems fine for now.

@mbakker7
Copy link
Copy Markdown
Contributor

This PR is replaced by #105.

@mbakker7 mbakker7 closed this May 11, 2026
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