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.

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

Feature/update variable estimates #421

Merged
merged 19 commits into from
Nov 18, 2019
Merged

Feature/update variable estimates #421

merged 19 commits into from
Nov 18, 2019

Conversation

Affie
Copy link
Member

@Affie Affie commented Oct 22, 2019

No description provided.

@Affie Affie requested a review from dehann October 22, 2019 15:37
Affie
Affie commented Oct 23, 2019
View reviewed changes
src/FactorGraph01.jl
@@ -737,6 +737,8 @@ function doautoinit!(dfg::T,
end
pts,inferdim = predictbelief(dfg, vsym, useinitfct, logger=logger)
setValKDE!(xi, pts, true, inferdim)
#TODO test
setVariablePosteriorEstimates!(xi)
Copy link
Member Author

Choose a reason for hiding this comment

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

@dehann, I think adding extra data breaks compare test, but I haven't had time to dig into the details.

Copy link
Member

Choose a reason for hiding this comment

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

these compare functions recently moved to DFG, so we should just tie up there too -- thanks!

@Affie Affie mentioned this pull request Oct 23, 2019
Closed
dehann
dehann approved these changes Oct 23, 2019
View reviewed changes
Copy link
Member

@dehann dehann left a comment

Choose a reason for hiding this comment

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

understood as adding PPE update to doautoinit, and pending concern about compare functions detecting change in unit testing that has not yet been addressed. Also, that the compare functions are moving to DFG.

@dehann dehann added this to the v0.8.0 milestone Oct 23, 2019
@dehann
Copy link
Member

dehann commented Oct 23, 2019

Hi @GearsAD , just mention you to keep mental note prior to DFG v0.5.0 -- that we may need to modify some of the compare stuff either in IIF or DFG when adding this PR. I think this PR should land in the IIF v0.8.0 release, unless there are any objections.

@Affie
Copy link
Member Author

Affie commented Oct 24, 2019

Lets hold off on this one and do it all in one go pending JuliaRobotics/DistributedFactorGraphs.jl#147 (comment) I think we are close enough to a final decision.
I use it currently, but I'm happy working on a branch for now.

@Affie Affie requested a review from dehann October 25, 2019 14:35
@Affie
Copy link
Member Author

Affie commented Oct 25, 2019

needs JuliaRobotics/DistributedFactorGraphs.jl#183

@dehann
Merge pull request #434 from JuliaRobotics/bug/missingimportandskipes…
aea4fa0 
…timate

Fix for tests - note that having different estimates will fail in compare
dehann
dehann approved these changes Nov 18, 2019
View reviewed changes
Copy link
Member

@dehann dehann left a comment

Choose a reason for hiding this comment

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

added several updates from the original PR

@dehann dehann merged commit 148d1f0 into master Nov 18, 2019
@dehann
Copy link
Member

dehann commented Nov 19, 2019

addresses #214

@dehann dehann deleted the feature/updateVariableEstimates branch June 15, 2020 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dehann dehann dehann approved these changes

Assignees
No one assigned
Labels
feature request needs testing upstream
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Affie @dehann @GearsAD