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

TreeMsgLikelihoods should swap NamedTuple for Tuple #1573

Open
dehann opened this issue Jul 26, 2022 · 2 comments
Open

TreeMsgLikelihoods should swap NamedTuple for Tuple #1573

dehann opened this issue Jul 26, 2022 · 2 comments

Comments

@dehann
Copy link
Member

dehann commented Jul 26, 2022

Could have a similar compile time implication as was found in:

@Affie
Copy link
Member

Affie commented Jul 26, 2022

Are you referring to sender in this?

mutable struct LikelihoodMessage{T <: MessageType} <: AbstractPrior
sender::NamedTuple{(:id,:step),Tuple{Int,Int}}
status::CliqStatus
belief::Dict{Symbol, TreeBelief} # will eventually be deprecated
variableOrder::Vector{Symbol}
cliqueLikelihood::Union{Nothing,SamplableBelief} # TODO drop the Union
msgType::T
hasPriors::Bool
# this is different from belief[].inferdim, as the total available infer dims remaining during down msgs -- see #910
childSolvDims::Dict{Int, Float64}
# calc differential factors for joint in the child clique
jointmsg::_MsgJointLikelihood
# diffJoints::Vector{NamedTuple{(:variables, :likelihood), Tuple{Vector{Symbol},DFG.AbstractRelative}}}
end

It is good as a NamedTuple.

@dehann
Copy link
Member Author

dehann commented Jul 29, 2022

Looking at .sender I think that should be okay, since the type is hard coded.

Not recalling exactly, but upon looking through this again I think it was actually the NamedTuple in the MsgRelativeType const that might be causing the Bayes tree compile time delays.

Just quickly, I made a very small runtime improvement:

But does notyet get rid of the potential NamedTuple compile time issue on tree message passing.

I'm also worried about the non-concrete union type on .cliqueLikelihood

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants