P/frequency dependent sensitivity#443
Conversation
|
The rebasing is throwing me for a loop. Could I get some help with the commit cleanup? |
|
Sure thing, heads up that I will force-push to this branch shortly. |
ebrahimebrahim
force-pushed
the
p/frequency_dependent_sensitivity
branch
from
8dc2859 to
2caf959
Compare
peterhollender
force-pushed
the
p/frequency_dependent_sensitivity
branch
from
2caf959 to
e00eaba
Compare
|
@peterhollender Want me to review this yet? If so, which makes sense to review and merge first, this one or #444 ? (Before the next slicer extension release, I am thinking we should get in all of these big openlifu-python changes, so that we can test/upgrade things in the extension all at once.) |
ebrahimebrahim
left a comment
ebrahimebrahim
left a comment
There was a problem hiding this comment.
This is great!
A couple of minor points and this should be good to go.
Co-authored-by: Ebrahim Ebrahim <ebrahim.ebrahim@kitware.com>
ebrahimebrahim
left a comment
ebrahimebrahim
left a comment
There was a problem hiding this comment.
Looks great! Opening #448 for later is fine.
One thing to still be super careful about and that you might want to address: np.interp assumes the freqs are sorted monotonically. Sorting is not currently enforced anywhere, e.g. you could do it in Element.__post_init__.
ebrahimebrahim
force-pushed
the
p/frequency_dependent_sensitivity
branch
from
be6b46a to
7d6222a
Compare
|
(adding the sorting befroe merging actually) |
Needed so that np.interp in sensitivity_at_frequency produces correct results.
Co-authored-by: Ebrahim Ebrahim <ebrahim.ebrahim@kitware.com>
2 participants
closes #439 #440