Skip to content

Conversation

ywkim0606
Copy link
Contributor

@ywkim0606 ywkim0606 commented May 17, 2023

Summary

TableFactor is a discrete probabilistic factor that can replace DecisionTreeFactor.
It should support all features that the current DecisionTreeFactor has such as multiplication, division, sum, and max.
It was developed for faster MAP inference on CSP and Classical Planning problems where discrete state space is very sparse.
Unittest includes a runtime benchmark between TableFactor and DecisionTreeFactor given various sparsity.
Supports cardinality up to 18446744073709551615.

What is the solution?

Relies on Eigen3's SparseVector data structure. Multiplication and division use the tensor contraction method proposed in SPARTA.

What areas of GTSAM does it impact?

gtsam/discrete

How to test?

run provided unit test (testTableFactor.cpp)

Other Notes

In-depth implementation details are in thesis's chapter 4.

Runtime comparison between DecisionTreeFactor and TableFactor on Macbook pro M1 chip.

image

@ywkim0606 ywkim0606 marked this pull request as ready for review May 17, 2023 08:28
@ywkim0606 ywkim0606 marked this pull request as draft May 17, 2023 08:34
@ywkim0606 ywkim0606 marked this pull request as ready for review May 17, 2023 08:39
@ywkim0606
Copy link
Contributor Author

@dellaert

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.

Some comments

using Binary = std::function<double(const double, const double)>;

public:
/** The Real ring with addition and multiplication */
Copy link
Member

Choose a reason for hiding this comment

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

somehow thought we already had that in a base class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its in AlgebraicDecisionTree. I didn't want to include it into TableFactor but I we can use the one implemented in AlgebraicDecisionTree.

@dellaert
Copy link
Member

I don't see any changes yet - send me an email when I should start CI again.

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.

Amazing work, Yoonwoo!
I will merge, as all unit tests succeed. @varunagrawal there is a possibility this could speed up Hybrid inference by a lot.

@dellaert dellaert merged commit 213543d into borglab:develop Jun 7, 2023
@arutkowski
Copy link
Contributor

FYI, this appears to use boost/format even if boost features are disabled in cmake.

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