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

maint and Common CSM, fix MixtureRelative #929

Merged
merged 5 commits into from
Sep 30, 2020
Merged

Conversation

dehann
Copy link
Member

@dehann dehann commented Sep 29, 2020

Hi @Affie , note I moved some CSM functions into a common_consolidate file. Most of these functions should be agnostic to the solver being used, and likely a few more in CSM.jl that could move over too. Just wanted to move some over to get the story started. Check if you can add these to the take! model system perhaps please. If both solvers enter and exit CSM the same, and have a few functions in common, then that really helps limit the scope of differences between fetch and take models.

ref #855

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2020

Codecov Report

Merging #929 into master will decrease coverage by 1.12%.
The diff coverage is 58.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #929      +/-   ##
==========================================
- Coverage   62.66%   61.53%   -1.13%     
==========================================
  Files          50       52       +2     
  Lines        4952     4971      +19     
==========================================
- Hits         3103     3059      -44     
- Misses       1849     1912      +63     
Impacted Files Coverage Δ
src/ApproxConv.jl 95.10% <ø> (-0.29%) ⬇️
src/CliqueTypes.jl 88.88% <ø> (+41.83%) ⬆️
src/CompareUtils.jl 16.66% <0.00%> (-33.34%) ⬇️
src/FactorGraph.jl 70.26% <0.00%> (ø)
src/IncrementalInference.jl 80.00% <ø> (ø)
src/JunctionTreeTypes.jl 22.14% <ø> (-0.23%) ⬇️
src/TreeDebugTools.jl 16.73% <0.00%> (+16.73%) ⬆️
src/Deprecated.jl 7.89% <13.88%> (+2.22%) ⬆️
src/SolverAPI.jl 69.51% <16.66%> (-2.65%) ⬇️
src/CSMCommon_Consolidate.jl 45.71% <45.71%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bede36...fe8908b. Read the comment docs.

@Affie
Copy link
Member

Affie commented Sep 29, 2020

I tried to find the common points and so far it looks like the best places might be:

  • blockUntilChildrenHaveStatus_StateMachine <==> waitForUp...
  • waitChangeOnParentCondition_StateMachine <==> waitForDown...

blockUntilChildrenHaveStatus_StateMachine fits well and the 2 are (can be) roughly equivalent up to there.

@dehann dehann changed the title maint and common CSM maint and Common CSM, fix MixtureRelative Sep 30, 2020
@dehann
Copy link
Member Author

dehann commented Sep 30, 2020

Hi Johan, that sounds right. Probably also

  • slowIfChildrenNotUpsolved <==> waitForUp

@dehann dehann merged commit 31a0d17 into master Sep 30, 2020
@dehann dehann deleted the maint/3Q20/general16 branch January 5, 2021 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants