Uncertainties for constrained parameters #1682
Conversation
Update fitted parameters values with uncertainty objects and propagate them to constrained parameters
Add uncertainty objects with 0 standard deviation to non fitted parameters. This makes sure that parameter value attribute is always an uncertainty object
|
Presume someone has to add the dependency package here so it gets into release & developer code. |
|
Have now done pip install uncertainties, so can build locally on Windows. See log below. 11:04:06 - ERROR: Traceback (most recent call last): |
|
So now I am slightly puzzled, but may not be clever enough, in my local build I did But M1.sld_shell = 8.2588 (0.006352) I was expecting a simple linear combination of the sigma here, |
It is a linear combination for variance, so it should be sqrt((0.6*0.0094)**2 + (0.4*0.00722)**2) :-)
Parameter correlation is not taken into account for the uncertainty calculation. It thus assumes parameters are independent. |
|
In which case I am concerned that the package is not doing what most people expect for independent variables. (I did check, that as its docs say M1.sld_solvent = M2.sld_solvent - M2.sld_solvent gives zero with error zero. Which is also possibly not good., unless it is actaully tracking the algebra properly. ) |
|
Re: formating, use backticks around code and triple-backticks around code blocks. This will keep asterisks as Re: results: I don't know how you are calculating 8.2947. Uncertainty via forward Monte Carlo matches the results of the uncertainty package: It differs from SasView in the fourth digit , presumably because of limited precision in the intermediate printed representation. This also matches the analytical variance as given on wikipedia: It is actually tracking the algebra properly, so |
|
As Nicholas mentioned, the code does not encode the parameter covariance. It turns out that this would be pretty easy to do: https://pythonhosted.org/uncertainties/user_guide.html#use-of-a-covariance-matrix With dream, the covariance could be computed directly from the sample instead of using the Hessian approximation. |
|
OK, they didn't teach it like that back in my "high school" days. I suppose the simple view is that combining the two observations results in a smaller fractional uncertainty as there are now more observations. I am happy now, so will approve. If we could put in the covariances also, that would be very good. [I don't know how you are calculating 8.2947 - sorry must have mistyped in my calculator, at least twice over] |
Take into account parameter correlation when computing the uncertainties
|
I've updated the code so parameter covariance is taken into account when computing the uncertainties |
src/sas/sascalc/fit/BumpsFitting.py
Outdated
| # TODO: move uncertainty propagation into bumps | ||
| # TODO: should scale stderr by sqrt(chisq/DOF) if dy is unknown | ||
| values, errs, state = result['value'], result["stderr"], result[ | ||
| 'uncertainty'] |
There was a problem hiding this comment.
The call to problem.cov must have the returned fit values inserted into the model. Need to guarantee that is the case even if the fit is being run in a different thread. It will be safer to add cov to the result object at the same time that stderr was added since we know the model is in the correct state.
Then we can write:
from uncertainties import ufloat, correlated_values
from bumps.parameters import unique as unique_parameters
...
# Turn all parameters (fixed and varying) into uncertainty objects with zero uncertainty.
for p in unique_parameters(problem.model_parameters()):
p.value = ufloat(p.value, 0)
# Update varying parameters with fitted uncertainty, preserving correlations.
fitted = correlated_values(result['value'], result['cov'])
for p, v in zip(self._parameters, fitted):
p.value = v
# Propagate correlated uncertainty through constraints.
problem.setp_hook()There was a problem hiding this comment.
I still have to separate the case where only one parameter is being fitted since np.cov returns a 0 dimension array when a (1,n) array is passed as argument. This will cause correlated_values to fail since it is excepting at least a 1-d array.
Have the result dict return the covariance matrix to make sure that the probem is in the correct state when computing the matrix
|
In this branch today, fitting 3 sets simultaneously, but NO constraints anywhere. I am seeing errors as below, will try to work out what kicked them off... 12:16:39 - INFO: 2020-10-13 12:16:39 12:28:12 - ERROR: Eigenvalues did not converge 12:28:12 - ERROR: Traceback (most recent call last): |
|
Loaded & set up 3 fitpages, did a simultaneous fit OK, then a fit on fitpage1 OK, then another simultaneous fit - Error, slightly different to the one above: 12:39:54 - ERROR: Eigenvalues did not converge Some further single fits on the three fitpages then work, some give errors. (Also why is residuals plot jumping up from where I parked it out of the way.) |
Could you send me the project causing the error? I can't figure out if the problem originates from some malformed covariance matrix or if it's something else... |
|
It may depend on the particular version of LAPACK installed:
That'll make it kind of hard to debug. I tried to reproduce it with an ill-conditioned matrix and with a non-symmetric matrix, but no luck. Even a matrix of ones didn't throw an error. If you print out the covariance matrix on error by changing this code to (untested): import logging
try:
fitted = correlated_values(values, cov)
except:
fitted = [ufloat(v, dv) for v, dv in zip(values, errs)]
with np.printoptions(precision=15):
logging.error(f"Parameter correlations not propagated. Covariance failed with cov=\n{cov}")then we can at least see what kind of matrices cause problems. If we only do this if there are derived parameters, then we won't flood the sasview log file, but it'll take longer to debug. |
|
Problem seems to come from negative values in the covariance diagonal which then fill the correlation matrix corresponding column and row with |
|
In my cases that gave the errors there were not any constraints set up, I just used "fit" on the simultaneous fit tab to run three fitpages at once. So perhaps code should not be trying to use the covariances in such circumstance? |
|
There are a couple of "solutions" but need to do something as it is used in all fits, not just constrained. |
|
@pkienzle suggests using diagonal rather than NaN as an option. Agreed. |
|
In this one try "fit" on fitpage2 this one has too many params fitting, so there will be extreme correlation. The fit engine does not always come up with the error, so depends what route the fit takes to get there. In this one, "fit" on Simult Fit tab In my local build on Win10 of ESS_GUI_uncertainties both will give errors. (I am presuming that the L-M engine is clever enough to re-invert the least squares matrix, without the Marquardt modification to the diagonal, before extracting the parameter variances etc, else if the fit has "converged" with a large marquardt lambda value, poor results (too small?) will appear for the variances & covariances.) |
|
Some related issues: |
Check if constraints are applied before calling `correllated_values`
Set covariance to `np.nan` if covariance computation fails
Ignore covariance in case `correlated_values` call raises `LinALgError` and disable error propagation in case it raises a `TypeError` (which is raised when using `math` functions).
|
Hey guys I've started looking at this PR. This is the error message in receiving: I've uploaded the files in txt format, and tried converting them to json. |
|
Hi @Caddy-Jones , have you tried renaming the file to .json (the .txt may have been added to add it to Github) and loading it as an analysis or project from the File menu? Just a thought... |
|
That works, thanks :) |
|
Hi @RichardHeenan, are you still experiencing the same errors with cov_errors_failingjson.txt and cov_errors_failing2json.txt ? Because I am not getting any error messages when running the fits for either of these files. I am wondering if the commits by the previous intern have fixed the problem. |
hi @Caddy-Jones , when I load test projects as above into a local build of this branch, I no longer get error messages, and some uncertainties are appearing on the constrained parameters, so that is good! The project loader is alas not overwriting Fitpage1, but adds FitPage2 through FitPage4 (Later I will try some freshly generated constrained fits and see what I can break, meanwhile you may as well merge all this and then we see if anyone notices the changes) |
|
If I understand correctly then @RichardHeenan , we are happy to merge this now but there may be some other issues that need to be ticketed separately? It looks like @Caddy-Jones has fixed the conflicts already so it is ready to merge otherwise. |
Undoing changes from PR #1682
This PR aims at having uncertainties computed for constrained parameters and displayed in the GUI. The errors are popagated using the uncertainties package, as suggested by @pkienzle during the last biweekly call.