Closed yvrob closed 1 month ago
Yves,
First, just to make sure we're on the same page: the PDSampler as I implemented it in the main Chrono library (from where it was copied in this DEM-E project) is random. It is just that the underlying random number engine is always seeded with the same value (0) in the constructor of ChPDSampler (this is the new name for this class in Chrono after a recent refactoring).
However, a user could always change this seed value, by a simple call:
chrono::utils::rengine().seed(my_seed);
right after the construction of a ChPDSampler object.
For convenience, I just pushed (to the main Chrono repository) a minor change that adds a function SetRandomEngineSeed
to ChPDSampler (to make sure a change in seed can only be done after the construction of the ChPDSampler object, else it wouldn't have an effect). So, now you would do:
chrono::utils::ChPDSampler<float> sampler(my_radius);
sampler.SetRandomEngineSeed(my_seed);
This convenience function could also be added to the copy of PDSampler in the DEM-E code.
By the way, I understand that some users may want to use a different seed at each different run, but I don't agree that that should be hardcoded in any way. Being able to reproduce results is important (e.g. in debugging). It should be up to the user to seed the engine however they want and however is appropriate for their problem (e.g., using time of day, host ID, /dev/random/, etc, etc). This has always been possible using the first method above.
Hi Yves,
You can use SetRandomEngineSeed directly in DEME as well now.
Ruochun
Closing the issue
Hi Ruochun,
Thank you so much for letting me know!
Best regards, Yves Robert
On Tue, Jul 30, 2024 at 9:09 PM Ruochun Zhang @.***> wrote:
Hi Yves,
You can use SetRandomEngineSeed directly in DEME as well now.
Ruochun
— Reply to this email directly, view it on GitHub https://github.com/projectchrono/DEM-Engine/issues/6#issuecomment-2259445038, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANNXOHAZTUYWEUBFW6ZNILDZPA2LHAVCNFSM6AAAAABDXIAK2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJZGQ2DKMBTHA . You are receiving this because you authored the thread.Message ID: @.***>
The DEM-Engine implementation of
PDSampler
seems to have a flaw: every time we call it again, it has the same random state. Indeed, when the user calls theSampleCylinderZ
for instance (observed with other sampling functions), the same set of random positions is sampled. The minimal example shows the issue.The printed text is a series of "56", meaning that the length of the sampled positions is always the same. The obtained figure is the following:
We can clearly see that each set of points overlaps the others. The expected behavior would be something like the one shown in a (less efficient) Python sampler I made (prints 55, 58, 53, 54, ... as lengths):
Additional suggestion: an additional boolean argument to
DEME.PDSampler
called (or similar to) keep_history that would allow the users to choose if they want to keep the previously sampled positions or erase the history every time a new sampling is applied.