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

Suspicious behavior of TUnuran::SampleMulti #10222

Closed
1 task done
DingXuefeng opened this issue Mar 24, 2022 · 2 comments
Closed
1 task done

Suspicious behavior of TUnuran::SampleMulti #10222

DingXuefeng opened this issue Mar 24, 2022 · 2 comments
Assignees
Labels
affects:6.26 bug fixathon This issue can be tackled at a ROOT fixathon in:Math Libraries

Comments

@DingXuefeng
Copy link

DingXuefeng commented Mar 24, 2022

  • Checked for duplicates

Describe the bug

It seems that to sample a N-dimensional vector, we need to call this method N times.

See log file:

for (int j = 0; j < 20000; j++) {
    hzmodel.Sample(&flux_real[0]);
    if(j<10)
    std::cout<<j<<" "<<flux_real[0]<<" "<<flux_real[1]<<" "<<flux_real[2]<<std::endl;

output

1 5.73992 4.99711 4.88
2 5.73992 4.99711 5.26624
3 5.72944 4.99459 5.25179
4 5.34123 4.99459 5.25179
5 5.34123 4.96792 5.25179
6 5.34123 4.96792 5.2696
7 5.26477 4.99232 5.52042
8 5.59451 4.99232 5.52042
9 5.59451 5.04631 5.52042

Expected behavior

By calling SampleMulti, all components should be randomized.

To Reproduce

try ROOT tutorial

Setup

tested on
ubuntu 20.04 + ROOT v6.26.00 (wget)
ubuntu 20.04 + ROOT v6.24.06 (wget)
MacOS 10.15 + ROOT v6.26.00 (brew)

Additional context

N/A

@DingXuefeng
Copy link
Author

It seems deeper problems exists. In my study, I need to sample a 3D multi-variate gaussian distribution. For comparison, I set correlation to zero, so that I can simply sample three independent variables. The results using unuran and the one using three indepent variables are incompatible, even with above mentioned dirty fix.

Currently I switch to alternative methods, yet it might be useful to understand what is the problem.

I think some unit test and integration test should be implemented.

@lmoneta
Copy link
Member

lmoneta commented Sep 19, 2022

The problem is that the default method Unuran uses for multi-dimensional distribution (hitro) is based on Markov-Chain MC so it is expected to observe this behaviour if you look at the single events, instead, you should use at the generated obtained distribution. See for example https://search.r-project.org/CRAN/refmans/Runuran/html/hitro.new.html
If you switch unuran (in the TUnuran::Init) to use the method vnrou then it will be fine when looking at the single events generated.
Before closing this issue, I think we need to improve the documentation in TUnuran for this.

@vepadulano vepadulano added the fixathon This issue can be tackled at a ROOT fixathon label Feb 5, 2024
@ferdymercury ferdymercury changed the title Suspicous behavior of TUnuran::SampleMulti Suspicious behavior of TUnuran::SampleMulti Feb 6, 2024
lmoneta added a commit to lmoneta/root that referenced this issue Mar 5, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT root-project#10222
dpiparo pushed a commit that referenced this issue Mar 8, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT #10222
@dpiparo dpiparo added this to Issues in Fixed in 6.32.00 Mar 8, 2024
@dpiparo dpiparo closed this as completed Mar 8, 2024
jblomer pushed a commit to jblomer/root that referenced this issue Mar 13, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT root-project#10222
kristupaspranc pushed a commit to kristupaspranc/root that referenced this issue Apr 10, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT root-project#10222
lobis pushed a commit to lobis/root that referenced this issue Apr 10, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT root-project#10222
kristupaspranc pushed a commit to kristupaspranc/root that referenced this issue May 21, 2024
…ions

The default method for chaning multi-dim distributions is now `vnrou` instead of the Markov CHain based method hitro, which generates (being a MC MC) correlated values and not iid.

Update also documentation and tutorial

This fixes ROOT root-project#10222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:6.26 bug fixathon This issue can be tackled at a ROOT fixathon in:Math Libraries
Projects
Status: Done
Development

No branches or pull requests

5 participants