Closed yetongumich closed 1 year ago
Any reason this hasn't been reviewed yet? If the speedup potential is promising, I need it for the legged state estimator.
Did some clean up by
@gchenfc @varunagrawal @danbarla Would you like to do a review of the updated PR and see if it is ready to merge in?
@gchenfc @varunagrawal Can either of you approve this PR so that we can merge it into the master?
@yetongumich I'll review it by tonight.
@varunagrawal @ProfFan It seems a refactorization in gtsam
eliminates boost
, which makes GTDynamics
fail. Do we want to address it in this PR or a different PR?
@dellaert mentioned in a private conversation that we should consider using release/4.2 temporarily (e.g. in CI) while we migrate.
I think it (migrating) should definitely be a separate PR since there's lots of unrelated code in GTDynamics that uses boost::optional etc. As for updating the CI to use release/4.2, I guess still a separate PR probably makes the most sense... (my personal opinion)
Sounds good! I think I'll just leave the PR as it is since it cannot pass the CI check.
@yetongumich why not go ahead and make a PR against this branch with the updated CI? I already fixed some of the compile issues against 4.2.
Approving since we can make changes along the way.
@yetongumich I used this PR to remove additional boost stuff.