PDSampler should be random

[yvrob]

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 the SampleCylinderZ for instance (observed with other sampling functions), the same set of random positions is sampled. The minimal example shows the issue.

import DEME
import matplotlib.pyplot as plt
for i in range(10):
    sampler = DEME.PDSampler(0.1)
    pos = sampler.SampleCylinderZ([0,0,0], 0.5, 0)
    x = [p[0] for p in pos]
    y = [p[1] for p in pos]
    plt.scatter(x,y, label=i, alpha=0.2)
    print(len(pos))
plt.legend(loc='center left', bbox_to_anchor=[1.05,0.5], title='Batch #')

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.

[rserban]

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.

[Ruochun]

Hi Yves,

You can use SetRandomEngineSeed directly in DEME as well now.

Ruochun

[Ruochun]

Closing the issue

[yvrob]

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 <#6 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANNXOHAZTUYWEUBFW6ZNILDZPA2LHAVCNFSM6AAAAABDXIAK2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJZGQ2DKMBTHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>