-
Notifications
You must be signed in to change notification settings - Fork 27
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
RF: Use scipy.interpolate.BSpline to construct spline basis #393
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #393 +/- ##
==========================================
+ Coverage 80.53% 80.56% +0.03%
==========================================
Files 26 26
Lines 2296 2290 -6
Branches 286 287 +1
==========================================
- Hits 1849 1845 -4
+ Misses 402 401 -1
+ Partials 45 44 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
6061011
to
d17bd89
Compare
Co-authored-by: Chris Markiewicz <[email protected]>
Use a construction that will potentially allow us to calculate derivatives more easily.
d17bd89
to
a22f76c
Compare
a22f76c
to
58ecbe7
Compare
@oesteban This should be a relatively quick review. Apart from the specific B-spline invocation change, the main change is that our design matrices are transposed from before. The effect is that we can stop transposing them after generation. The tests verify that the |
Read through with fresh eyes. I'm going to go ahead and merge. |
This builds on and replaces #388. The additional padding broke our ability to reconstruct the field, as we were estimating more parameters but not actually saving them.
This PR maintains the current functionality and paves the way for implementing the Jacobian (#391) by using
BSpline()
objects directly instead of throughBSpline.design_matrix()
. I also restore the use of awithin_support
mask, which significantly decreases the runtime ofunwarp_wf.resample
nodes. There is little effect onbs_filter
nodes, as we downsample to a low enough resolution to make the fit process very quick already.