Skip to content

Conversation

@phipsgabler
Copy link
Member

Implements what I proposed in #46. Each tilde or dot_tilde call becomes (dot)_tilde_{assume,observe}, respectively, depending on position. And particularly, the varname and indices are also passed in for observations of non-literals.

These additional arguments are just ignored by default, which makes the changes backwards-compatible.

Additionally, I changed the way assume/observed is distinguished by the @preprocess macro, which is now @isassumption and does return a boolean. Thus the dispatch on tuple types becomes unnecessary.

@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #53 into master will increase coverage by 0.01%.
The diff coverage is 97.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   80.20%   80.22%   +0.01%     
==========================================
  Files          12       12              
  Lines         864      900      +36     
==========================================
+ Hits          693      722      +29     
- Misses        171      178       +7     
Impacted Files Coverage Δ
src/model.jl 78.94% <ø> (-2.01%) ⬇️
src/context_implementations.jl 52.17% <83.33%> (+1.41%) ⬆️
src/compiler.jl 94.08% <100.00%> (+0.57%) ⬆️
src/utils.jl 56.60% <100.00%> (+15.57%) ⬆️
src/varname.jl 82.75% <100.00%> (-10.10%) ⬇️
src/varinfo.jl 87.61% <0.00%> (+0.03%) ⬆️

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 534826c...63ca3f4. Read the comment docs.

Copy link
Contributor

@mohamed82008 mohamed82008 left a comment

Choose a reason for hiding this comment

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

Looks good overall. I left one comment. Thanks for the great PR!

Copy link
Contributor

@mohamed82008 mohamed82008 left a comment

Choose a reason for hiding this comment

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

LGTM

@mohamed82008 mohamed82008 merged commit 7879726 into TuringLang:master Apr 3, 2020
@mohamed82008
Copy link
Contributor

Thanks @phipsgabler!

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.

3 participants