Skip to content
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

Reworked piecewise polynomial implementation in Sequence to use scipy's PPoly #140

Merged
merged 2 commits into from
Oct 23, 2023

Conversation

FrankZijlstra
Copy link
Collaborator

@FrankZijlstra FrankZijlstra commented Oct 23, 2023

This implements calculate_kspacePP using scipy's PPoly object, which behaves very similar to the Matlab implementation. This is obviously much cleaner, and also much faster. A helper function get_gradients is also available to immediately get access to the waveforms in PPoly format.
I renamed the function to calculate_kspace, because this function was no longer functional, and not intended to be used anymore anyway. I added a deprecation warning for calculate_kspacePP, but this function could also just be removed altogether. This also resolves #107.

Furthermore, I split waveforms_and_times into three functions: waveforms, adc_times, and rf_times. This allows efficient access to for example only the excitation times (e.g. when only calculating TE/TR). waveforms_and_times remains compatible with the previous implementation, but rf_times returns excitation/refocusing times and frequency/phase in separate arrays (instead of the non-intuitive tfp_ variables).

Further small changes:

  • Improved performance of calculating of tc in calculate_kspace (especially in sequences with ramp-sampling)
  • Changed block_durations in Sequence to be a dictionary, to avoid potential issues when using non-incremental block IDs (previous code assumed this was the case)
  • Fixed waveforms_export (though this function seems a bit superfluous with the other ways to access this information)

…s `PPoly`

- Changed `calculate_kspacePP` into `calculate_kspace` (added a deprecation warning for `calculate_kspacePP`)
- Separated code for generating `PPoly` objects into `get_gradients`
- Separated `waveforms_and_times` into three functions: `waveforms`, `adc_times`, and `rf_times` to efficiently allow access to individual components on demand
- Added `time_range` argument to these functions to get only information from a specific part of the sequence
- Improved performance of calculating of `tc` in `calculate_kspace` (especially in sequences with ramp-sampling)
…s when inserting blocks with manually chosen block IDs
@sravan953
Copy link
Collaborator

Thanks for this! I spent a little time on the calculate k-space function but could not get the Scipy version to behave as expected. I ended up translating the MATLAB function to make it work, but that ended up being slow. Glad you got this to work!

@sravan953 sravan953 merged commit 94474c0 into imr-framework:dev Oct 23, 2023
@schuenke schuenke mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Sequence' object has no attribute 'gradient_waveforms' in Sequence.calculate_kspace
2 participants