-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Class interface to fit powder spectra #174
Conversation
2f96143
to
4bb8254
Compare
Change powspec to call spinwave() once to take advantage of chunking or mex threading Refactor sw_freemem() to poll memory max only every 10s Change sw_egrid() to not calculate Bose factor for T=0 (default) The above changes save ~30% time per iteration in a powder fit (now spinwave() eval is ~50% of time per iteration, was ~30%)
Currenlty uses lsqnonlin to do the minmisation - this is in the optimization toolbox so not availible to all. The plan is to replace this with a call to an optimiser of choice (including some packaged with spinw)
4bb8254
to
6a4de70
Compare
And pass model parameters as a row vector as requried by matparser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a couple of minor points. Also, you can delete the fitpow.m
file...
But it needs more documentation and some basic tests... (maybe just make sure that the example syntax can run with 1 or 2 iteration so that it doesn't significantly increase the time to run tests...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry another doc request and also a request to add a "feature" from fitpow
- of caching a previous powspec
calculation (from a previous iteration). This is only useful for gradient methods like L-M but it saves evaluating the expensive powder spinwave calculations when the algorithm is varying the background or scaling parameters - it effectively saves around (n_bgpar/n_totalpars) fraction of iterations.
Thanks @mducle for the suggestion of caching the spinwave calculation, it does provide considerable speedup when using e.g. the
As expected the speedup is greater for the independent backgrounds as there are more background parameters. |
COuld possibly be made more concise, but at least is more user friendly and can adjust parameters accross multiple slices with one call now.
c282bc2
to
c7e8704
Compare
Thanks for the detailed review!
|
Set to default true in sw_fitpowder
5d4adc4
to
589d70d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #174 +/- ##
==========================================
+ Coverage 40.51% 42.11% +1.60%
==========================================
Files 240 240
Lines 15981 16079 +98
==========================================
+ Hits 6474 6771 +297
+ Misses 9507 9308 -199 ☔ View full report in Codecov by Sentry. |
@RichardWaiteSTFC Looks good but could you add the changes in the following diff, please: The changes to The two lines to invalidate the cache are needed because Edit: This is mainly a problem if you call |
Thanks Duc - there is already a method called |
Doh! Sorry I should have read the code more carefully. Yes, using |
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)