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.

Sign up for GitHub

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

Jump to bottom

why do we create twice the CrossvalidationSplitting objects #4783

Open
vigsterkr opened this issue Oct 23, 2019 · 5 comments · May be fixed by #4784
Open

why do we create twice the CrossvalidationSplitting objects #4783

vigsterkr opened this issue Oct 23, 2019 · 5 comments · May be fixed by #4784
Labels
Type: Bug

Comments

@vigsterkr
Copy link
Member

shogun/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h

Lines 198 to 206 in 40f40f0

auto instance_x=new CCrossValidationSplitting(new CBinaryLabels(dummy_labels_x), m_num_folds);
auto instance_y=new CCrossValidationSplitting(new CBinaryLabels(dummy_labels_y), m_num_folds);
random::seed(instance_x, prng);
random::seed(instance_y, prng);
m_kfold_x=unique_ptr<CCrossValidationSplitting>(instance_x);
m_kfold_y=unique_ptr<CCrossValidationSplitting>(instance_y);
random::seed(m_kfold_x.get(), prng);
random::seed(m_kfold_y.get(), prng);

@vigsterkr
Copy link
Member Author

ok i've just realised that the second time it is just wrapped by a unique_ptr (i read it for some reason std::make_unique), still i'm not so sure why do we seed it twice? :)

@vigsterkr
Copy link
Member Author

@theartful why do we seed there twice the same object?

@ghost
Copy link

ghost commented Oct 23, 2019

@vigsterkr It's a mistake on my part. Using std::make_unique and seeding once is enough.

@vigsterkr
Copy link
Member Author

@theartful ok thnx for confirming!
@lambday seems we need to update unit tests for xval mmd

vigsterkr added a commit to vigsterkr/shogun that referenced this issue Oct 23, 2019
@vigsterkr
seeding fix for xvalmmd
9939a8d 
fix shogun-toolbox#4783
@vigsterkr vigsterkr linked a pull request Oct 23, 2019 that will close this issue
Open
vigsterkr added a commit to vigsterkr/shogun that referenced this issue Oct 25, 2019
@vigsterkr
seeding fix for xvalmmd
7022a56 
fix shogun-toolbox#4783
@stale
Copy link

stale bot commented Apr 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 20, 2020
@stale stale bot removed the stale label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

seeding fix for xvalmmd vigsterkr/shogun
2 participants
@vigsterkr @lambday