Add ability to fit individually, globally, or both #63
Conversation
|
I've switched the minimizer used for the multifit from Steepest Descent to Levenberg Marquardt - this significantly improves the global E1 estimate for the case where invalid spectra haven't been excluded from the fit, and means that the system test no longer fails. |
jess-farmer
force-pushed
the
specify_fit_type
branch
from
787f3f0
to
66d0f1b
Compare
jess-farmer
changed the base branch from
0_seperate_script_into_modules
to
59_5_manually_enter_invalid_detectors
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_analysis.py
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_analysis.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_analysis.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_analysis.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_analysis.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_fit.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_fit.py
Outdated
Show resolved
Hide resolved
unpackaged/vesuvio_calibration/calibration_scripts/calibrate_vesuvio_fit.py
Outdated
Show resolved
Hide resolved
| rel_tol_theta, rel_tol_L1 = self._extract_tolerances(deepcopy(tolerances)) | ||
| theta_errors = self._assert_theta_parameters_match_expected(params_table, rel_tol_theta) | ||
| L1_errors = self._assert_L1_parameters_match_expected(params_table, rel_tol_L1) | ||
| L1_errors = None | ||
| if fit_type != 'Shared': |
There was a problem hiding this comment.
Remind me, why can't we test L1 parameters if the fit is shared?
There was a problem hiding this comment.
I've tried writing tests for the global L1 values (which are calculated using the global E1 values), but found that the values didn't match the expected L1 parameters - I was going to bring this up with Matthew when we next speak to him to check that the global L1 values are being calculated in the correct way
There was a problem hiding this comment.
Ok sounds good, I'll take another look at this PR when I get a chance, we're currently in our release sprint so I have a bit of a to-do list!
|
Thanks for the review @MialLewis - I've now addressed most of your comments and have pushed the changes I've made so far. I still need to add more tests, and will aim to get this done by the end of the week. |
Description of work:
Add ability to specify whether peaks should be individually fit, globally fit, or both, as mentioned in #59
To do:
To test:
Run the calibration scripts with the different
SharedParameterFitTypeoptions:'Individual','Shared', and'Both'