imr-framework / pypulseq

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

Output sequence waveforms from .seq files #29

Closed tonggehua closed 2 years ago

tonggehua commented 4 years ago

Enhancement Description It would be helpful to be able to output entire waveforms (i.e. all blocks concatenated together) for:

Describe the solution you'd like A Class function for seq objects. It would be nice to be able to specify (1) Time range and (2) Block index range for custom outputs for examination and debugging.

Describe alternatives you've considered N/A

sairamgeethanath commented 4 years ago
  1. csv file format for spectrometer
  2. Separate files for RF and Gradients and a way to combine them and delay periods
sravan953 commented 4 years ago

@tonggehua For gradient waveforms, pypulseq has the seq.gradient_waveforms() method. Let me know if that's sufficient for gradient waveforms.

tonggehua commented 4 years ago

@sravan953 Right now, everyone will have to write their own code for outputting the entire waveforms. It would be valuable to have an inbuilt method for it. Plus, it might just be a simple modification from seq.plot().

sravan953 commented 4 years ago

@tonggehua If you have a seq object, call seq.gradient_waveforms() to get the gradient waveforms across the three channels. Does that clarify what the method does?

tonggehua commented 4 years ago

@sravan953 It does output the entire gradient waveforms. So what we need is adding on the RF, ADC, and uniform timing across. Thanks!

sravan953 commented 4 years ago

@tonggehua Yes, thanks for confirming! Will work on the RF and ADC. Could you elaborate on what you mean by 'uniform timing'? Thanks again.

sairamgeethanath commented 4 years ago

I would not recommend uniform timing. I would recommend event-based timing. I do not see value in having times for delays (0s), only start and stop should suffice. If we need a common time clock, that will run at 1us and can be quite lengthy and redundant.

tonggehua commented 4 years ago

@sairamgeethanath For compression and storage, uniform timing is definitely not efficient. The feature I am suggesting is having flattened outputs that share the same time vector for (1) easy plotting and (2) benchmarking (comparing one sequence to another) purposes.

sairamgeethanath commented 4 years ago

@tonggehua Not efficient for plotting too. I need two endpoints to connect and make a straight line. Even from a console standpoint, you just sample value and HOLD until it changes: either for writing or for reading.

tonggehua commented 4 years ago

@sairamgeethanath @sravan953 For the .seq waveform output feature, I would like to use it for more robust benchmarking of seq2xml. JEMRIS's output is uniformly timed across RF, G, and ADC. I would like to be able to compare the .h5 output and seq.output_waveforms(), say, directly. Having the same time resolution (maybe Δt should be user-specifiable) helps.

sairamgeethanath commented 4 years ago

@tonggehua @sravan953 This feature needs to return two outputs: one should be in the format compatible with JEMRIS (@tonggehua please share a sample for gre with @sravan953 ). The other output should be compatible with the console needs of event-based logging only.

sravan953 commented 2 years ago

@tonggehua Commit f36f91a7dd7ff7c0b37ce0f559312c918f63afe4 contains an updated waveforms_and_times() method - https://github.com/imr-framework/pypulseq/blob/cc4aa8d4a50b2a17d4e375306d5d322e70ceac75/pypulseq/Sequence/sequence.py#L1096. Let me know if it's what you were looking for?

tonggehua commented 2 years ago

@sravan953 It seems to do the same thing; I will try plotting with this function and get back to you. Please do consider my pull-request (#62) as it also has the feature of specifying the time range before generating the waveforms.

sravan953 commented 2 years ago

@tonggehua Alright will wait for your feedback.

sravan953 commented 2 years ago

PR #62 takes care of this, closing!