Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent noise from going out of bounds #114

Merged
merged 7 commits into from
Aug 20, 2023
Merged

Prevent noise from going out of bounds #114

merged 7 commits into from
Aug 20, 2023

Conversation

lobis
Copy link
Member

@lobis lobis commented Aug 8, 2023

lobis Ok: 68

Closes #113

@lobis lobis requested review from jgalan and juanangp August 8, 2023 13:50
@lobis lobis added the bug Something isn't working label Aug 8, 2023
@lobis lobis marked this pull request as ready for review August 8, 2023 14:03
@@ -591,12 +595,14 @@ void TRestRawSignal::GetWhiteNoiseSignal(TRestRawSignal* noiseSignal, Double_t n
double* dd = new double();
uintptr_t seed = (uintptr_t)dd + (uintptr_t)this;
delete dd;
TRandom3* fRandom = new TRandom3(seed);
TRandom3 random(seed);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the seed should be stored inside TRestRawSignal::fSeed. It should be initialised just once, not every time GetWhiteNoiseSignal is called. This may lead to unreproducible results.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 9499c8e.

By default it will use the same seed as the global seed for TRandom but it can also be specified via TRestRawSignal::SetSeed.

@lobis lobis merged commit 1bef356 into master Aug 20, 2023
@lobis lobis deleted the lobis-fix-noise branch August 20, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TRestRawSignalAddNoiseProcess not working correctly near limits
2 participants