Support for regrowing fits from seeds. #209
Conversation
edcallaghan
left a comment
edcallaghan
left a comment
There was a problem hiding this comment.
A couple of small comments, but overall this looks completely reasonable to me.
| dtime = newpiece.range().end(); | ||
| double tstart = std::max(domain->begin(), oldpiece.range().begin()); | ||
| double tend = std::min(domain->end(),oldpiece.range().end()); | ||
| if(tstart < tend){ |
There was a problem hiding this comment.
I'm having trouble understanding the effect of this branch. When I draw out what I think is happening, I think that the pieceRange() call guarantees that this condition is met (unless the underlying pieces are themselves pathological). If that's correct, then the branch isn't necessary. If it's incorrect, then should there be an else-clause to non-silently handle that situation?
Fit/Track.hh
Outdated
| for(auto const& hit : hits){ | ||
| tmin = std::min(tmin,hit->time()); | ||
| tmax = std::max(tmax,hit->time()); | ||
| if(hit->active() || !active){ |
There was a problem hiding this comment.
If just marked as const then the compiler may help with this as-is, but reordering these conditions (or moving the check on the functionally-const active to outside of these loops) would be an optimization for when active == false.
2 participants
This includes the updates to support cloning, as well as some subsequent fixes.