-
Notifications
You must be signed in to change notification settings - Fork 26
Description
I tried using our OrientedSource to do some final checks/timings for #1167 and I had to make several hot fixes to avoid redundantly computing the common lines matrix, rotations, and all the associated intermediary sync voting calculations. It became very obvious for large problems... During development most of my work was calling the orientation estimator (and its methods) directly, so I narrowly avoided seeing this.
Essentially calling estimate_shifts totally recomputes the entire CL algorithm. I knew it used rotations, but there is no reason to recompute the whole algo...
Then the MeanEstimator totally recomputes the entire CL algorithm again. Double yikes.
There was another place I think I am forgetting as well (that may have been a consequence of me trying to time estimate_rotations, I forget).
Anyway, in the end for a simple recon pipeline with shifts, the current code computes the CL algorithm 2-3 times instead of once.
I'm going to create a PR after 1167 that fixes this, probably will mix with the other OrientedSource cleanup that needs to happen. It was only a few lines, I just want to keep it separate.