Opened 5 years ago

Last modified 5 years ago

#1204 assigned defect

5.0 not giving same fit results as 4.2

Reported by: richardh Owned by: piotr
Priority: blocker Milestone: SasView 5.0.0
Component: SasView Keywords:
Cc: Work Package: SasView QA and testing

Description

Load e.g. AOT_microemulsion-shell_contrast.xml
Fit e.g. just scale & length with cylinder model, taking care that all params are at their default values (this may involve switching model category and re-selecting cylinder to get them back if you have done other things first).

4.2 and 5.0 do not give the same fit results for a "poor fit" such as this. They should do if using the same fit engine, and same fit options.
I suspect an initialisation or default for weighting, resolution, fit engine or something else is not working properly but I have not yet managed to find the culprit.

For "good" fits the parameter values and their errors are very close but still not identical as they perhaps ought to be.

Attachments (5)

coil1.png (222.5 KB) - added by richardh 5 years ago.
starting parameters
coil2.png (219.8 KB) - added by richardh 5 years ago.
turn on 2 params
coil3.png (224.3 KB) - added by richardh 5 years ago.
more params fitting
coil4_iQwt.png (226.4 KB) - added by richardh 5 years ago.
change from di weights to I(Q) weights
coil5.png (229.1 KB) - added by richardh 5 years ago.
same results with default dI weighting IF start at same place

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by richardh

PS there are times when 4.2 and 5.0 do give the same results, so there is something intermittent here, I am beginning to suspect the "set weighting" flag in 4.2.

comment:2 Changed 5 years ago by richardh

  • Resolution set to invalid
  • Status changed from new to closed

The set weighting choice in 4.2 seems to have no effect, whereas it does in 5.0, I am going to close this ticket for now write a new one for 4.2

Changed 5 years ago by richardh

starting parameters

Changed 5 years ago by richardh

turn on 2 params

Changed 5 years ago by richardh

more params fitting

Changed 5 years ago by richardh

change from di weights to I(Q) weights

comment:3 Changed 5 years ago by richardh

  • Resolution invalid deleted
  • Status changed from closed to reopened

Some more direct comparisons between 4.2 and 5.0, see screen shots of same fits in each giving different answers, 5.0 on left, 4.2 on right.
These for AOT_microemulsion shell contrast, fit to mono_gauss_coil

The initial start up model looks the same, but the residuals plot is rather different!
None of the fit results are identical, and are very different when change to I(Q) weights.

Suspect something is perhaps not being initialised properly, or passed properly???

This time I was in the beta_approx branch, not master, of sasmodels, but I don't think that mattters.

(Apologies I should not have turned on both scale and I_zero here as they are redundant in this model, however the conclusions are the same).

Last edited 5 years ago by richardh (previous) (diff)

Changed 5 years ago by richardh

same results with default dI weighting IF start at same place

comment:4 Changed 5 years ago by richardh

In default dI weighting can get 5.0 and 4.2 to give same answer, as per latest attached screen shot, IF start at the same place, but this is not the case in the other weighting schemes.

comment:5 Changed 5 years ago by richardh

Looks like there was a long standing issue in 4.x (and possibly even 3.x), see #1205
Now need to check weighting scheme effect in 5.0 and 4.2 side by side again

comment:6 Changed 5 years ago by richardh

  • Priority changed from critical to blocker

I have just downloaded 5.0 beta 2 on my desktop, and 4.2.1 build 493 from Jenkins on my laptop, if I load the same test data and run the same fits I am still not getting identical fit results (especially if the fit is poor) so there is I think still something wrong with the weightings in one or other of the code versions.

comment:7 Changed 5 years ago by richardh

Looks like "none" and "use dI data" are getting confused in both 4.2 and 5.0 depending the order in which you hit the buttons. Switching between them may give the same or different answers, so suspect there is an underlying issue in the logic of the code in both??? (sorry got to go home now)

comment:8 Changed 5 years ago by richardh

  • Owner changed from richardh to piotr
  • Status changed from reopened to assigned

(1) Further comparison of 5.0 beta2 against 4.2.1 suggests that 5.0 is starting up with weighting as "none" not the "Use dI data" that the gui claims. So I am going to assign this ticket for Piotr to have a look please. Thus by default 5.0 is not currently giving identical fits.

(2)The value of scale that is fitted in 5.0 is not the same as in 4.2.1 - were we expecting it to change? (In the example I used it was ~44% of the 4.2.1 value)

(3) In 4.2 the values of the reduced chai squared varies hugely with weighting scheme, but remains roughly comparable in 5.0. In this case I think 5.0 is correct in that our docs only have error squared in the equation for chai squared, thus no dependence on weights.

(4) In the options tab of 5.0, when you change weighting scheme the fit buttons briefly goes red and shows "running" as is it it is doing a fit with the new weights, however it does not actually do a fit, so you do have to hit the fit button on either the options or the model tab. This is consistent with 4.2.1, a new choice of weighting only takes effect on the next fit. Should we change this? Probably not?

comment:9 Changed 5 years ago by richardh

Ignore (2) above, that was my mistake, scale factor does agree as expected.

comment:10 Changed 5 years ago by richardh

Regarding (3) above, in 4.2.1 the residuals plot is showing (obs-calc)/weight, which is what the residuals workspace also contains, whilst in 5.0 the residuals plot and workspace are of simply (obs-calc) without the weight. This likely explains the large numerical differences in chai squared values.

Thus apart from the "none" weights case the residuals plot in 4.2 looks very different to that in 5.0

Do we want 5.0 to keep this behaviour, or to match 4.2.1 ?

comment:11 Changed 5 years ago by richardh

Repeating parts of previous comments as I was not able to mention this on the call yesterday, what do the science team think regarding (3) below?

(1) Having fixed the non-active weights in 4.2 (see #1205) further comparison of 5.0 beta2 against 4.2.1 suggests that 5.0 is starting up with weighting as "none" not the "Use dI data" that the gui claims. So this ticket is assigned to Piotr to have a look please. Thus by default 5.0 is not currently giving identical fits.

(2) Resolved - my mistake.

(3) In 4.2 the values of the reduced chai squared varies hugely with weighting scheme, but they remain roughly comparable in 5.0. In this case I think 5.0 is correct in that our docs only have error squared in the equation for chai squared, thus no dependence on weights.

However in 4.2.1 the residuals plot is showing (obs-calc)/weight, which is what the residuals workspace also contains, whilst in 5.0 the residuals plot and workspace are of simply (obs-calc) without the weight. This likely explains the large numerical differences in chai squared values.

Thus apart from the "none" weights case the residuals plot in 4.2 looks very different to that in 5.0

Do we want 5.0 to keep this behaviour, or to match 4.2.1 ? in which case this needs a further 5.0 ticket.

comment:12 Changed 5 years ago by butler

I guess the FIRST question really is why did it get changed in 5.0? is it because of an oversight when typing code? a deliberate omission because it was not understood by the person doing that coding? or because there was some discussion that 4.x was behaving incorrectly?

I would argue that in the first instance it should behave "exactly the same" until there is a discussion to argue that the current implementation is wrong.

Certainly as we all know there are plenty of things in 4.x that were/could be incorrect (or not state of the art). Regarding this particular question, I will have to read up a bit on "official statistical definitions" of Chi2 but I think that If you have a data set with vastly varying uncertainties, any goodness of fit that does NOT somehow account for that (and maybe a simple normalization isn't the perfectly correct answer?) would be meaningless? … scientifically speaking :-)

comment:13 Changed 5 years ago by piotr

Cursory look at 4.2 and 5.0 sources for chi2 calculation doesn't show any discrepancies, so the lack of weight scaling must be a bug elsewhere in the code. It is a bug, since I was trying to achieve numerical equivalency between the two versions. Whether the scaling is correct or not, I will fix the issue in 5.0.

Note: See TracTickets for help on using tickets.