Some old code I had set a parameter as a distribution.
lws_distribution::Distributions.Normal{Float64} = Parameter()
And then I would sample it within the Mimi model equations:
v.lws_sea_level[t] = v.lws_sea_level[t-1] + rand(p.lws_distribution)
This code no longer works. I get ERROR: MethodError: no method matching copy(::Normal{Float64}). I’m assuming Mimi internally creates a copy of this distribution which no longer seems to work. Is there a workaround or alternative Mimi syntax I’m not aware of?
Hi @FrankErrickson, thanks for pointing this out. It looks like a Mimi bug, since you should be able to set a parameter as a Distribution type, but we hadn’t thought of that use case so we try to call copy and there is no copy method for Distribution (which is immutable so it doesn’t need one), as the error message shows. I can add a bug fix to this next week, if it’s time sensitive I can also give you a work around in the meantime!
Issue: https://github.com/mimiframework/Mimi.jl/issues/640
Hey @lrennels, thanks. Definitely not time sensitive, it’s just a minor component in BRICK. For now I’m just sampling outside the model and passing the uncertain values in as a Mimi parameter which works fine. Whenever it’s updated in Mimi, I can switch back to to original distribution version.
@FrankErrickson I’m taking a look at this now and want to recreate what you were doing to make sure I get supporting it right. What did you initially set the parameter to in your model (with the set_param call? Something like Distributions.Normal(0,1)?
I’m having some trouble recreating the problem, maybe you can send me the repo or a zip folder of the model if it’s not pushed to Github?
@lrennels Cool, thanks for looking into this. In the model component, I set the parameter as:
lws_distribution::Distributions.Normal{Float64} = Parameter()
From what I remember and given some of the package updates, it might also be the case that the Distributions bit should be removed.
The equation is basically just a cummulative sum of random samples, so (after setting an initial condition) it looks like:
v.lws_sea_level[t] = v.lws_sea_level[t-1] + rand(p.lws_distribution)
The create model file would then have something like:
set_param!(brick, :landwater_storage, :lws_distribution, Normal(0.0003, 0.00018))
This code used to work, but I tried various iterations of it and it just seems like Mimi is trying to copy the distribution which throws the error.
Ok so I was oddly able to recreate your problem when using the latest tagged version of Mimi (v0.9.4) but not on the master development branch so we might have fixed this bug accidentally in the meantime. You can either try out BRICK on latest Mimi version by typing add Mimi#master in the pkg REPL or just wait for our next release and try it out on that one. The latter is probably easier for you, but happy to do either, I can help if you’re not sure how to do the environments!
Hi @lrennels, just wanted to revive this with all the new Mimi updates.
I went to convert the BRICK code to set a distribution as a parameter type for the latest version of Mimi. In the component definition I have:
lws_distribution = Parameter{Distributions.Normal{Float64}}()
This seems to work ok if I’m just running the model files directly on my end. But if I export the BRICK get_model function as a module, it no longer works and throws errors saying Distributions aren’t recognized in this particular component.
Do you happen to know if this is either (i) an issue with Mimi and modules not communicating correctly when the parameter type comes from an external package or (ii) me just screwing up the module?
Thanks!
Hi Frank,
I used the dummy code below to test out this issue. It looks like due to scoping you need to put using Distributions in your file both outside the module and inside the module as I’ve done below. I posed this to the team in the meantime, since it is not intuitive and seems like something we should try to fix if possible.
using Distributions
module brick
using Mimi
using Distributions
@defcomp component1 begin
savingsrate = Parameter{Distributions.Normal{Float64}}()
pickrate = Variable(index=[time])
function run_timestep(p, v, d, t)
v.pickrate[t] = rand(p.savingsrate)
end
end
function get_model()
m = Model()
set_dimension!(m, :time, collect(2015:5:2110))
add_comp!(m, component1)
set_param!(m, :component1, :savingsrate, Distributions.Normal(1.0))
return m
end
end # module
1 Like
No problem, I’m asking the team why exactly that’s true and seeing if we can change it
Actually just set this up to have BRICK as a package. The code you shared (with using Distributions inside and outside the module) works when I just test the files locally on my computer.
But if I throw these updated files up onto Git and add then try to add BRICK as a package, it no longer works (I get a UndefVarError: Distributions not defined error and a Failed to precompile MimiBRICK error).
Just FYI, this isn’t a big deal. I can always just sample these outside the model and pass them in as a parameter (having the parameter be a distribution would just be slightly cleaner).
Oh strange ok, it would still be good for me to get to the bottom of it, where’s the GitHub repo? I can try it myself and also take a look at the .toml files. If it’s not shareable no problem, I can bring it up with the team on Friday.
@FrankErrickson I think we’ve fixed this! Sorry if I confused things before, you don’t need calls to using outside your module, just inside before the call to @defcomp like
module brick
using Mimi
using Distributions
@defcomp component1 begin
savingsrate = Parameter{Distributions.Normal{Float64}}()
pickrate = Variable(index=[time])
function run_timestep(p, v, d, t)
v.pickrate[t] = rand(p.savingsrate)
end
end
function get_model()
m = Model()
set_dimension!(m, :time, collect(2015:5:2110))
add_comp!(m, component1)
set_param!(m, :component1, :savingsrate, Distributions.Normal(1.0))
return m
end