Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding additional Unit3 support in the wrapper: PriorFactorUnit3, Values::insert/update/at(Unit3) #576

Merged
merged 2 commits into from
Nov 4, 2020
Merged

adding additional Unit3 support in the wrapper: PriorFactorUnit3, Values::insert/update/at(Unit3) #576

merged 2 commits into from
Nov 4, 2020

Conversation

tmcg0
Copy link
Contributor

@tmcg0 tmcg0 commented Oct 31, 2020

This very simple PR adds some additional functionality for the GTSAM wrapper for Unit3 types. Specifically:

  • adding PriorFactorUnit3 as a wrapper type
  • Adding Values methods update(), insert(), at() that support Unit3 types.

(apologies if there is a good reason this functionality is missing, or if it's added somewhere else that I've missed)

@dellaert
Copy link
Member

This is awesome, but check why some of the CI runs fail?

@tmcg0
Copy link
Contributor Author

tmcg0 commented Nov 1, 2020

Hmm. The code I added was really small and shouldn't have broken anything. I'll try to dig in to why the MacOS CI fails. A quick look at the CI logs makes it look like it's something else going on.

@ProfFan
Copy link
Collaborator

ProfFan commented Nov 2, 2020

It appears to be some compatibility problem on latest boost (which is what brew tracks). I don't have bandwidth now unfortunately.

@tmcg0
Copy link
Contributor Author

tmcg0 commented Nov 2, 2020

It appears to be some compatibility problem on latest boost (which is what brew tracks). I don't have bandwidth now unfortunately.

Yep. I don't have a Mac, so I likely won't be able to track this down. But it looks like the latest merged PR passed the MacOS CI. I'm a bit confused as how that was passing. Did brew track a new boost in the last four days?

@tmcg0
Copy link
Contributor Author

tmcg0 commented Nov 3, 2020

The failing CI in this PR should be fixed by the latest PR which fixed some boost compatibility in Brew. Thanks @ProfFan for fixing that!

@ProfFan
Copy link
Collaborator

ProfFan commented Nov 3, 2020

@tmcg0 Glad to help :)

Please merge in or rebase to latest develop to fix CI.

Merging in some brew fixes that had previously caused CI to fail
@tmcg0
Copy link
Contributor Author

tmcg0 commented Nov 3, 2020

Looks like one of the Windows CI timed out after 6 hours. Also looks like that's happening to other recent PRs. I assume this is a known issue or a problem with GitHub's servers?

@ProfFan
Copy link
Collaborator

ProfFan commented Nov 4, 2020

Since it's only the Debug version I think it's timing out. If that is the only blocker then I think it's fine.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!! Very useful :-) And you helped us debug the Mac issue.

@dellaert dellaert merged commit 8a34d4c into borglab:develop Nov 4, 2020
@tmcg0 tmcg0 deleted the expand-wrapper-Unit3-functionality branch November 5, 2020 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants