Skip to content

Remove 'boost' dependency from Lago #1440

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Feb 5, 2023
Merged

Remove 'boost' dependency from Lago #1440

merged 18 commits into from
Feb 5, 2023

Conversation

ankur-rc
Copy link

@ankur-rc ankur-rc commented Feb 5, 2023

Currently, Lago was using algorithms from the Boost Graph Library (BGL), most notably to compute the Minimum Spanning Tree.

This PR attempts to remove the dependency of BGL from Lago by making the following changes:

  • Factors out the kruskal routine from SubGraphBuilder into a free function
  • Creates a function findminimumSpanningTree in Lago which uses kruskal
  • Removes all references of BGL within Lago by not including the graph.h header

Ankur Roy Chowdhury added 17 commits February 4, 2023 20:41
factor out class methods 'weights' and 'kruskal' into free-functions
981d8e9
add unit test for 'assignWeights'
cc7ed2d
Factors out 'kruskal' into it's own header and adds tests
a8fa2f4
Cleanup unused variables from test
3b2fe0a
remove commented code section
64267a3
WIP: replace instances of calling the graph-inl version of 'findMinim…
b83261e 
…umSpanningTree' with the lago version
replaces all instances of calling the graph-inl version of 'findMinim…
a0ca68a 
…umSpanningTree' with the lago version
Some cleanup
4be4b97
Removes import 'graph.h' inside 'lago.h'
e29a0d3
removes unused argument 'ordering' from 'kruskal'
3f65cfb
fix testKruskal
b5a1e96
remove extraneous argument 'ordering' from SubGraphBuilder::buildTree
3b745d3
adds docstring
b20827b
Fix formatting
c8124ec
minor edit
d5ec65e
cleanup some formatting and comments
c3045c0
Removes extra space
b2fe6fa
@dellaert dellaert self-requested a review February 5, 2023 05:08
dellaert
dellaert approved these changes Feb 5, 2023
View reviewed changes
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.

Very cool ! Will add @lucacarlone as a reviewer so can weigh in with comments, but almost ready to merge after addressing mostly "nits" :-)

@dellaert dellaert requested a review from lucacarlone February 5, 2023 05:23
@ankur-rc ankur-rc requested review from dellaert and removed request for lucacarlone February 5, 2023 18:49
@dellaert
Copy link
Member

dellaert commented Feb 5, 2023

will kill all but required CI to unblock my CI experiments in other branch, then will merge

@dellaert dellaert merged commit e8bce8d into borglab:develop Feb 5, 2023
@dellaert
Copy link
Member

dellaert commented Feb 5, 2023

Congrats @ankur-rc on your first gtsam PR :-)

@dellaert dellaert added this to the Remove BOOST milestone Feb 6, 2023
@ankur-rc ankur-rc deleted the no_boost_lago branch February 17, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dellaert dellaert dellaert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Remove BOOST
Development

Successfully merging this pull request may close these issues.
2 participants
@ankur-rc @dellaert