Skip to content

Conversation

FredericWantiez
Copy link
Member

I think we need to propagate the reference particle in PG but sweep! is missing a ref parameter

@coveralls
Copy link

coveralls commented Jun 13, 2021

Pull Request Test Coverage Report for Build 933715358

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 55.163%

Files with Coverage Reduction New Missed Lines %
src/container.jl 1 90.98%
Totals Coverage Status
Change from base Build 895050782: -0.3%
Covered Lines: 203
Relevant Lines: 368

💛 - Coveralls

@codecov
Copy link

codecov bot commented Jun 13, 2021

Codecov Report

Merging #29 (5c5723e) into master (176640e) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
+ Coverage   55.43%   55.70%   +0.27%     
==========================================
  Files           5        5              
  Lines         368      368              
==========================================
+ Hits          204      205       +1     
+ Misses        164      163       -1     
Impacted Files Coverage Δ
src/container.jl 92.62% <100.00%> (+0.81%) ⬆️
src/smc.jl 97.22% <100.00%> (ø)

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 176640e...5c5723e. Read the comment docs.

@devmotion
Copy link
Member

Isn't this handled in

AdvancedPS.jl/src/smc.jl

Lines 104 to 113 in 176640e

# Create a new set of particles.
nparticles = sampler.nparticles
x = map(1:nparticles) do i
if i == nparticles
# Create reference trajectory.
forkr(state.trajectory)
else
Trace(model)
end
end
?

@FredericWantiez
Copy link
Member Author

FredericWantiez commented Jun 13, 2021

Was reading through resample_propagate, looks like we need to set ref to replace the selected particle in the last spot:
https://github.com/TuringLang/AdvancedPS.jl/blob/master/src/container.jl#L225-L229


# Perform a particle sweep.
logevidence = sweep!(rng, particles, sampler.resampler)
logevidence = sweep!(rng, particles, sampler.resampler, particles.vals[nparticles])
Copy link
Member

Choose a reason for hiding this comment

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

@yebai
Copy link
Member

yebai commented Jun 17, 2021

Many thanks @FredericWantiez - looks correct to me!

@yebai yebai merged commit c5440fb into TuringLang:master Jun 17, 2021
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.

4 participants