search

imr-framework / pypulseq

Pulseq in Python
https://pypulseq.readthedocs.io
GNU Affero General Public License v3.0
112 stars 58 forks source link

Bug fixes that produced incorrect test reports #125

Closed FrankZijlstra closed 11 months ago

FrankZijlstra commented 1 year ago

Fixed several bugs in Sequence and test_report that result in incorrect test reports (e.g. wrong slew rates!) and incorrect k-space trajectory calculation.

Checked test reports for gre, tse, gre_radial, ute, and epi_se_rs to be identical with MATLAB.

Additionally made some minor changes that avoid some warnings (bad conditioning in polynomial fit and division by zero).

btasdelen commented 1 year ago

Thank you for the PR @FrankZijlstra. That is quite a substantial fix, thanks for the effort!

The patch for the test report looks straightforward, but I am not really familiar with the polynomial codebase. Maybe @sravan953 can comment on that?

An aside: I am planning to propose a PR to change the internal representation of time to an integer in units of nanoseconds rather than float in units of seconds. That should hopefully resolve the issues regarding empirical selection of eps.

zhengliuer commented 11 months ago

An aside: I am planning to propose a PR to change the internal representation of time to an integer in units of nanoseconds rather than float in units of seconds. That should hopefully resolve the issues regarding empirical selection of eps.

This is really a great idea!

sravan953 commented 11 months ago

Hi all, apologies for the delay. I'll look into this in a day or two! Thank you for the PR @FrankZijlstra