Skip to content
New issue

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

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

Already on GitHub? # to your account

Modernize discrete factor graphs (a bit) #933

Merged
merged 4 commits into from
Nov 30, 2021
Merged

Conversation

dellaert
Copy link
Member

  • use emplace_shared were we can
  • bring use of Values up to date with rest of GTSAM (no shared pointer mania)

Technically an API change, but discrete is very seldomly used, if ever, except by me I think.

@dellaert dellaert added cleanup Help clean up old/obsolete aspects of GTSAM quick-review Quick and easy PR to review labels Nov 20, 2021
@dellaert dellaert requested a review from xsj01 November 20, 2021 21:37
CSP::sharedValues mpe = csp.optimalAssignment(ordering);
// GTSAM_PRINT(*mpe);
auto mpe = csp.optimalAssignment(ordering);
// GTSAM_PRINT(mpe);
CSP::Values expected;
insert(expected)(WA.first, 1)(CA.first, 1)(NV.first, 3)(OR.first, 0)(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for jumping in, but I remember last time you asked me to wrap some discrete in Python, and I realized this is not easy to wrap. Could we drop this syntax (which is boost specific) and use some C++11-only style instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

different PR

@xsj01
Copy link
Contributor

xsj01 commented Nov 30, 2021

Looks good to me

@xsj01 xsj01 merged commit deca3df into develop Nov 30, 2021
@xsj01 xsj01 deleted the feature/modernize_discrete branch November 30, 2021 23:16
@dellaert
Copy link
Member Author

Thanks @xsj01, but note reviewers don't merge :-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cleanup Help clean up old/obsolete aspects of GTSAM quick-review Quick and easy PR to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants