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

Consolidate old/new clique density selection #913

Closed
dehann opened this issue Sep 25, 2020 · 2 comments · Fixed by #923 or #926
Closed

Consolidate old/new clique density selection #913

dehann opened this issue Sep 25, 2020 · 2 comments · Fixed by #923 or #926

Comments

@dehann
Copy link
Member

dehann commented Sep 25, 2020

Make sure this is using the proper densities/potentials/likelihoods/factors stored in csmc.cliqSubFg -- i.e. make sure we are not using the old potentials list:

  • packFromLocalPartials!
  • packFromIncomingDensities!
  • packFromLocalPotentials!

This came up as part of the LikelihoodMessage.hasPriors discussion (#914)

cc @Affie

@dehann
Copy link
Member Author

dehann commented Sep 27, 2020

HI @Affie , all right so this issue has been resolved with #923 (and few preceding PRs too). I was looking at treeinit a bit and this is what's currently happening on Hex. Note that the "parent starting down solve too early bug" (#924) is also still present.

Here is the new result with .useMsgLikelihooods=false

Screenshot from 2020-09-26 23-35-33

Here is the result for tree init with .useMsgLikelihoods=true and remember that this now includes the msg.hasPriors story from Friday included as well:
Screenshot from 2020-09-26 23-35-40

So there are probably still two or three bugs rolling around in there somewhere. I'm going to look at resolving #754 and #924

@dehann
Copy link
Member Author

dehann commented Sep 27, 2020

also note that I deprecated the cliqGibbs function and a few of it's utilities. Think they were just making life difficult. Also note that the internal clique solve order of variables still needs to be updated away from the old btnd.potentials plumbing (#925)

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