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

DiscreteFactor multiply method #1963

Merged
merged 14 commits into from
Jan 6, 2025
Merged

DiscreteFactor multiply method #1963

merged 14 commits into from
Jan 6, 2025

Conversation

varunagrawal
Copy link
Collaborator

Add a multiply method for DiscreteFactor which can be used to handle derived classes easily.

One caveat I encountered is that I have to also make sum and max as abstract base class methods, and finally add new constructors for DiscreteLookupTable and DiscreteConditional which take DiscreteFactor as an input. I can do this in another PR for cleanliness sake.

@varunagrawal varunagrawal requested a review from dellaert January 5, 2025 18:59
@varunagrawal varunagrawal self-assigned this Jan 5, 2025
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.

Quick Look, more later

gtsam/discrete/DiscreteFactor.h Show resolved Hide resolved
gtsam/discrete/DecisionTreeFactor.h Outdated Show resolved Hide resolved
gtsam_unstable/discrete/AllDiff.h Outdated Show resolved Hide resolved
gtsam/discrete/TableFactor.cpp Show resolved Hide resolved
@varunagrawal varunagrawal requested a review from dellaert January 6, 2025 17:27
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.

Approve modulo comments

gtsam/discrete/DecisionTreeFactor.h Outdated Show resolved Hide resolved
gtsam/discrete/DecisionTreeFactor.h Outdated Show resolved Hide resolved
gtsam/discrete/DecisionTreeFactor.cpp Show resolved Hide resolved
gtsam/discrete/DecisionTreeFactor.cpp Show resolved Hide resolved
gtsam/discrete/DiscreteFactorGraph.cpp Show resolved Hide resolved
gtsam/discrete/DiscreteFactorGraph.cpp Show resolved Hide resolved
gtsam/discrete/TableFactor.h Outdated Show resolved Hide resolved
gtsam/discrete/TableFactor.cpp Show resolved Hide resolved
@varunagrawal varunagrawal merged commit 47074bd into develop Jan 6, 2025
33 checks passed
@varunagrawal varunagrawal deleted the discrete-multiply branch January 6, 2025 20:22
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.

2 participants