Soss.jl icon indicating copy to clipboard operation
Soss.jl copied to clipboard

Enable passing of rng parameter to random functions

Open sethaxen opened this issue 5 years ago • 6 comments

Once https://github.com/baggepinnen/MonteCarloMeasurements.jl/pull/58 is merged, the way will be paved to make Soss's calls to particles and rand optionally take a first argument rng::AbstractRNG. This will make Soss runs more reproducible and make the package easier to test.

I can handle adding the parameter to normal functions, but I could use some guidance on how to modify some of the special generated-style functions Soss has, as I'm not familiar with GeneralizedGenerated.jl. In particular, the functions in src/primitives/rand.jl.

sethaxen avatar Feb 01 '20 09:02 sethaxen

New release of MonteCarloMeasurements.jl has been tagged 😊

baggepinnen avatar Feb 01 '20 11:02 baggepinnen

Thanks, @baggepinnen!

sethaxen avatar Feb 01 '20 18:02 sethaxen

Sounds good!

@sethaxen, the functions for codegen have names Soss.source*. So for example,

function sourceRand() 
    function(m::Model)
        
        _m = canonical(m)
        proc(_m, st::Assign)  = :($(st.x) = $(st.rhs))
        proc(_m, st::Sample)  = :($(st.x) = rand($(st.rhs)))
        proc(_m, st::Return)  = :(return $(st.rhs))
        proc(_m, st::LineNumber) = nothing

        vals = map(x -> Expr(:(=), x,x),variables(_m)) 

        wrap(kernel) = @q begin
            $kernel
            $(Expr(:tuple, vals...))
        end

        buildSource(_m, proc, wrap) |> flatten
    end
end

There's some extra logic here to handle putting the pieces together in the right way, but the main thing to pay attention to is the proc subfunction. This walks through the Statements of the model and builds an expression for each.

So for rand, this will involve adding an argument to

proc(_m, st::Sample)  = :($(st.x) = rand($(st.rhs)))

From a quick look, I think particles is similar.

I'm tempted to just do this. But if you don't mind, I'd really like to improve our bus factor. What to take a shot at it? I can jump in if you run into trouble.

And thanks for bringing this up!

cscherrer avatar Feb 04 '20 04:02 cscherrer

I'm tempted to just do this. But if you don't mind, I'd really like to improve our bus factor. What to take a shot at it? I can jump in if you run into trouble.

Sure! I doubt I'll really increase your bus factor though. IR is foreign to me.

So for rand, this will involve adding an argument to

Hmm, there are a few ways I can see to do this.

  1. add rng as a positional argument to proc. This would require a change to buildSource, which requires proc have 2 positional arguments
  2. add rng as a keyword argument to proc. This would require adding kwargs... to all proc to slurp the keywords.
  3. add rng as a field of Sample. I think this would also require some changes to buildSource.

Option 2 looks to me like the best way to go. What do you think?

sethaxen avatar Feb 05 '20 02:02 sethaxen

IR is foreign to me. Me too! This is "just" Julia expressions, with some GeneralizedGenerated magic :)

What do you think? It just hit me that we can just hard-code this. I'm treating leading underscores as "private" variables, so we can probably just make the small change :($(st.x) = rand($(st.rhs))) -> :($(st.x) = rand(_RNG, $(st.rhs))).

That will help it generate an expression to put in the body of the function we're building.

Then we'll need to get set up to dispatch to it. so we'll need to mess with the methods a bit. Hmm... Let me see about this, then (if you don't mind) we can talk through it so you understand the changes.

Sorry I won't get to it tonight, been a long day and I need to decompress a bit

cscherrer avatar Feb 05 '20 04:02 cscherrer

It just hit me that we can just hard-code this. I'm treating leading underscores as "private" variables, so we can probably just make the small change :($(st.x) = rand($(st.rhs))) -> :($(st.x) = rand(_RNG, $(st.rhs))).

Oh yeah, then I definitely don't understand what's going on here.

Sorry I won't get to it tonight, been a long day and I need to decompress a bit

No worries! It's not urgent. 🙂

sethaxen avatar Feb 05 '20 04:02 sethaxen