PieterjanRobbe / GaussianRandomFields.jl

A package for Gaussian random field generation in Julia
Other
64 stars 11 forks source link

user input AbstractRNG in sample() #47

Closed odunbar closed 1 year ago

odunbar commented 1 year ago

Thanks for this neat package, I am looking to generate some GRFs and so this could save me time!

Quick Q

I notice the last merge is almost a year ago - is this still being maintained?

Issue

For reproducibility in automated tests, I will need to be able to sample the GRFs with a user-provided RNG rng::AbstractRNG. Is this perhaps why there is this xi input to the current sample(grf,xi) function?

In any case, it may be a nice solution if instead one extends julia Statistics/Distributions sample function to give

Cheers, Ollie

odunbar commented 1 year ago

If I might add to this issue, I think there is another benefit of extending the StatsBase sample function instead of defining your own.

In many stats packages, the user may be defining their own extension of StatsBase.sample function for their own objects. This will cause warnings on calling using GaussianRandomFields which introduces a new unknown sample function into the workspace on precompile. Currently one can only import GRF to avoid these warnings. I think this might disappear if GRF also extended StatsBase.sample

This is a somewhat annoying design decision from the Julia devs but I think we must live with this, in various Julia discussion boards it is apparently encouraged (though nowhere really stated) that users should always look into extending XYZBase type packages where they exist for common functions.

PieterjanRobbe commented 1 year ago

Thanks for the suggestion, I've added the functionality in b5aa780e and 7e509e30.

odunbar commented 1 year ago

Awesome, Thanks very much!