Optimize MAG L1D functions to reduce SPICE tranform overhead #2554
Conversation
subagonsouth
left a comment
subagonsouth
left a comment
There was a problem hiding this comment.
Glad that you got some improvement. I suspect that the you could get another modest bump based on one of my comments.
I'd be interested to hear what you tried for multi-processing. I have some vague ideas about how we might be able to implement something that would get the spice multi-processing to work. I am thinking along the lines of running each of the transforms from J2000 -> DYNAMIC_FRAME in a separate process.
|
|
||
| # Nominally, this is expected to create MAGO data. However, if the configuration | ||
| # setting for always_output_mago is set to False, it will create MAGI data. | ||
| l1d_norm.rotate_frame(ValidFrames.J2000) |
There was a problem hiding this comment.
I know that it isn't quite a clean, but I think you would see another incremental speed up by first rotating to the SRF frame, storing that data and then going to J2000 before doing the rest as you are.
As implemented, you are not taking advantage of the fixed rotation code you added for going direct from MAGI/MAGO to SRF.
There was a problem hiding this comment.
That's because actually, at this point in the process, the frame is DSRF, not MAGO. Internally, the code spins from MAGO/MAGI -> SRF -> DSRF as part of processing, so that fixed rotation code is being used as part of l1d_norm generation.
There was a problem hiding this comment.
I see. Ok, so there is still some room for optimization.
- You could store the SRF values the first time that transform occurs.
- You could try to avoid doing the SRF -> DSRF. That transform traverses two dynamic frame transforms: SRF -> J2000 and then J2000 -> DSRF.
I will approve so you can get this optimization released but it might be worth capturing this additional possibility in a ticket.
| xr.Dataset | ||
| The resulting dataset. | ||
| """ | ||
| l1d_copy = deepcopy(l1d_input) |
There was a problem hiding this comment.
Is this deep copy of the MagL1d object copying lots of data other than the vectors that get rotated? It might be less expensive to just store the J2000 cordinates rather than copying the entire object.
There was a problem hiding this comment.
The vast majority of the data in the classes is the vectors/timestamps. It is true that there is some unnecessary copying going on here, but... since there is a fair bit of logic about MAGO/MAGI and selecting which one to use inside each class, I am hesitant to pull that out and duplicate it here.
| if ( | ||
| start_frame in (ValidFrames.MAGO, ValidFrames.MAGI) | ||
| and end_frame == ValidFrames.SRF | ||
| ): | ||
| # Since Instrument -> spacecraft rotations are static, we can optimize | ||
| # by only passing one time into frame_transform. | ||
| single_mago_epoch = self.epoch_et[0] | ||
| single_magi_epoch = self.magi_epoch_et[0] |
There was a problem hiding this comment.
This logic looks good to me.
That's basically what I did. I used the concurrent.futures framework to basically run each of the instances inside the |
subagonsouth
left a comment
subagonsouth
left a comment
There was a problem hiding this comment.
Nice incremental speedup.
Interesting. Did you do any kernel furnishing as part of a setup for each process? |
No, I didn't specifically furnish kernels |
2 participants
Change Summary
This addresses #2518 and includes some fixes described in that ticket.
Overview
This showed about a 20% improvement.