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

Add a Sample transducer #39

Merged
merged 1 commit into from
May 21, 2020
Merged

Add a Sample transducer #39

merged 1 commit into from
May 21, 2020

Conversation

devmotion
Copy link
Member

I don't know if this PR should ever be merged, it's mainly me playing around with and learning about transducers. I assume transducers could be useful in AbstractMCMC since it's easy to combine and nest them with good performance (see, e.g., JuliaLang/julia#33526). Some rudimentary progress logging can be achieved with Transducers.withprogress. Parallel reductions (multicore or multithreaded) can be achieved easily, and it's also easy to only save every nth element, drop samples, or iterate until some criterion is met.

Currently the implementation feels a bit weird since the transducer is mutated, ideally stepping would not mutate the model or the sampler (not sure how to deal with the RNG) (see #31).

@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #39 into master will increase coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   97.54%   97.76%   +0.22%     
==========================================
  Files           5        6       +1     
  Lines         122      134      +12     
==========================================
+ Hits          119      131      +12     
  Misses          3        3              
Impacted Files Coverage Δ
src/AbstractMCMC.jl 100.00% <ø> (ø)
src/transducer.jl 100.00% <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 c7efc8a...14831b8. Read the comment docs.

@cpfiffer
Copy link
Member

Neat! I've always wanted to see some transducers stuff in here. I'm not opposed to merging it in.

As to the RNG, I wonder if you could generate a seed at each step, pass it forward, and use the seed to make a new RNG on a per-step basis. Though maybe this is costly to do. Either way I'd like to see these be fully immutable/stateful.

@devmotion
Copy link
Member Author

As to the RNG, I wonder if you could generate a seed at each step, pass it forward, and use the seed to make a new RNG on a per-step basis. Though maybe this is costly to do. Either way I'd like to see these be fully immutable/stateful.

I guess one possibility would be to make the RNG part of the private state of the transducer, then you don't have to create a RNG in every step. The question still remains what to do in the initial step. One could save a seed and an RNG as template, and then initialize a copy of the RNG with the seed in the initial step. That should then give the exact same samples every time - but I'm not sure if we actually want that and if the initialization step is too costly.

Copy link
Member

@cpfiffer cpfiffer left a comment

Choose a reason for hiding this comment

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

Happy to merge this one over and figure out what to do with it later, if/when stateful sampling becomes more of a thing.

@devmotion
Copy link
Member Author

Yeah, I guess we can just let it evolve and grow and not export or use it properly right now.

@cpfiffer cpfiffer merged commit 509b5cf into master May 21, 2020
@delete-merged-branch delete-merged-branch bot deleted the transducer branch May 21, 2020 15:58
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.

2 participants