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. |
8dc2859 to
2caf959
Compare
Closes #435
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
There was a problem hiding this comment.
This is great!
A couple of minor points and this should be good to go.
| def sensitivity_at_frequency(sensitivity: float | List[tuple[float, float]], frequency: float) -> float: | ||
| if isinstance(sensitivity, list): | ||
| freqs, values = zip(*sensitivity) | ||
| freqs = np.array(freqs, dtype=np.float64) | ||
| values = np.array(values, dtype=np.float64) | ||
| if frequency in freqs: | ||
| idx = np.where(freqs == frequency)[0][0] | ||
| return float(values[idx]) | ||
| else: | ||
| return float(np.interp(frequency, freqs, values, left=values[0], right=values[-1])) | ||
| return float(sensitivity) |
There was a problem hiding this comment.
This can be simplified a lot, because np.interp will do the right thing
| def sensitivity_at_frequency(sensitivity: float | List[tuple[float, float]], frequency: float) -> float: | |
| if isinstance(sensitivity, list): | |
| freqs, values = zip(*sensitivity) | |
| freqs = np.array(freqs, dtype=np.float64) | |
| values = np.array(values, dtype=np.float64) | |
| if frequency in freqs: | |
| idx = np.where(freqs == frequency)[0][0] | |
| return float(values[idx]) | |
| else: | |
| return float(np.interp(frequency, freqs, values, left=values[0], right=values[-1])) | |
| return float(sensitivity) | |
| def sensitivity_at_frequency(sensitivity: float | List[tuple[float, float]], frequency: float) -> float: | |
| if isinstance(sensitivity, list): | |
| freqs, values = zip(*sensitivity) | |
| return float(np.interp(frequency, freqs, values)) | |
| return float(sensitivity) |
(also the floating point equality check freqs == frequency can be fragile -- but anyway it doesn't matter because the interp handles things correctly)
(Note that np.inter requires freqs to be monotonically increasing. If that's not guaranteed then you'd need to sort it at the top of this function.)
| @@ -173,9 +176,10 @@ def run_simulation(arr: xdc.Transducer, | |||
| dst_pos = np.array(positions[dst_idx]) | |||
| dist = np.linalg.norm(src_pos - dst_pos) | |||
| if dist <= arr.crosstalk_dist: | |||
| crosstalk_arr.elements += [arr.elements[dst_idx].copy()] | |||
| crosstalk_mat = np.vstack((crosstalk_mat, arr.crosstalk_frac*source_mat[src_idx,:])) | |||
| arr = crosstalk_arr | |||
| # crosstalk_arr.elements += [arr.elements[dst_idx].copy()] | |||
| crosstalk_mat[dst_idx,:] += arr.crosstalk_frac*source_mat[src_idx,:] | |||
| #crosstalk_mat = np.vstack((crosstalk_mat, arr.crosstalk_frac*source_mat[src_idx,:])) | |||
| #arr = crosstalk_arr | |||
| source_mat = crosstalk_mat | |||
| karray = get_karray(arr, | |||
| translation=array_offset, | |||
There was a problem hiding this comment.
clean up and remove commented out code? committing to the new way of computing crosstalk
| FOCAL_GAIN_LUT = xa.DataArray.from_dict( | ||
| {'dims': ('f0', 'crosstalk'), | ||
| 'attrs': {}, | ||
| 'data': [[2.807589054107666, | ||
| 3.2286391258239746, | ||
| 3.649686813354492, | ||
| 3.8181092739105225, |
There was a problem hiding this comment.
Totally optional but consider saving the lookup table into a data file (json, csv, etc.) to make the source code file shorter.
If you chose to do that:
- you would read the file here using
importlib.resourcesto locate it - you would ensure it's bundled int he install by adding it to the
[tool.setuptools.package-data]section. in case hatchling doesn't bnundle it automatically
closes #439 #440