Closed RichardWaiteSTFC closed 3 months ago
4 files ± 0 112 suites +4 4m 40s :stopwatch: - 4m 41s 711 tests + 25 693 :white_check_mark: + 25 18 :zzz: ±0 0 :x: ±0 2 016 runs +100 1 980 :white_check_mark: +100 36 :zzz: ±0 0 :x: ±0
Results for commit d72280cd. ± Comparison against base commit 3e8b492d.
:recycle: This comment has been updated with latest results.
2D plot of initial guess
1D plot of initial guess
Thanks @mducle for the suggestion of caching the spinwave calculation, it does provide considerable speedup when using e.g. the lm3
minimiser. Here are timings for fitting the 3 x1D cuts in the PR description
Background With/Without Cache
planar 142/223 s
indep. 104/247 s
As expected the speedup is greater for the independent backgrounds as there are more background parameters.
Thanks for the detailed review!
FYI having added fastmode
and neutron_output
to powspec
I ran 1 iteration of the simplex minimiser on the 2D dataset - here are the rough timings:
Time (s) fastmode neutron_output
45.6 1 1
60.1 0 1
69.3 0 0
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Attention: Patch coverage is 65.13158%
with 106 lines
in your changes missing coverage. Please review.
Project coverage is 42.11%. Comparing base (
735d30a
) to head (d72280c
). Report is 31 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
swfiles/sw_fitpowder.m | 66.66% | 80 Missing :warning: |
swfiles/@spinw/powspec.m | 76.92% | 9 Missing :warning: |
swfiles/sw_instrument.m | 0.00% | 9 Missing :warning: |
swfiles/+ndbase/lm3.m | 0.00% | 5 Missing :warning: |
swfiles/sw_mex.m | 0.00% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@RichardWaiteSTFC Looks good but could you add the changes in the following diff, please:
The changes to lm3.m
is a quick hack to make sure that "fixed" parameters stay fixed... we'll need to revisit this and rewrite lm3
properly but I need this for the current fitting stuff with the user.
The two lines to invalidate the cache are needed because add_data
and apply_energy_mask
changes the shape of the y
and e
members such that calc_background
will calculate a bg
for the new shape but if the cache is not invalidated it will return the old y
values (shape) and give a shape mismatch error.
Edit: This is mainly a problem if you call add_data
or apply_energy_mask
after a call to estimate_scale_factor
as that does a spinwave
calculation and saves it to the cache.
@RichardWaiteSTFC Looks good but could you add the changes in the following diff, please:
The changes to
lm3.m
is a quick hack to make sure that "fixed" parameters stay fixed... we'll need to revisit this and rewritelm3
properly but I need this for the current fitting stuff with the user.The two lines to invalidate the cache are needed because
add_data
andapply_energy_mask
changes the shape of they
ande
members such thatcalc_background
will calculate abg
for the new shape but if the cache is not invalidated it will return the oldy
values (shape) and give a shape mismatch error.Edit: This is mainly a problem if you call
add_data
orapply_energy_mask
after a call toestimate_scale_factor
as that does aspinwave
calculation and saves it to the cache.
Thanks Duc - there is already a method called clear_cache
- would it be OK to call that rather than touch the obj.model_params_cached
(which is currently private)? It deletes the ycalc
array so there's a bit of overhead re-assigning the memory but maybe that would have to happen anyway is the shape changes? I can change the behaviour of that anyway if you prefer?
Doh! Sorry I should have read the code more carefully. Yes, using clear_cache
is much better!
Doh! Sorry I should have read the code more carefully. Yes, using
clear_cache
is much better!
No worries, it was a good spot about cropping the data and the cache!
Note includes commits from #167 and #163
To-Do:
ndbase.lm2 & 3
andndbase.pso
that give incorrect size error when used as optimizer~~To test (data here)