You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
fcma: test_mvpa_voxel_selection fails with scikit-learn 0.22 #448
On Sun, 8 Dec 2019, 14:02 CameronTEllis, ***@***.***> wrote:
Is anyone working on this? I can take a crack at it if no one else is
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#448?email_source=notifications&email_token=AABINKV4UMPW6CMIA757JQDQXVVF5A5CNFSM4JVMGCBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGHLIBI#issuecomment-563000325>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABINKXDGK5NHDP2Y2B5NZ3QXVVF5ANCNFSM4JVMGCBA>
.
@CameronTEllis, I think it is better to contine the conversation from PR #437 here. Your explanation makes sense; it is likely the length difference is caused by a change in accuracy. Nevertheless, all but one of the accuracy values are the same. Therefore, I think it should be possible to preserve the accuracy value comparison in the test.
Indeed the difference between the results are similar but that is incidental as far as I can tell. Again, I am not sure why the value comparison is the appropriate test because if so it sets an extreme standard that should affect all of our tests. If it is necessary to include this test then we can put in an if statement that deals with this different computation. Other than that I am unsure how to proceed
You are right, we have no standard for tests requiring such comparisons. However, I see no harm keeping this existing test, as long as it is working, and it seems close to working to me.
My proposal, as written in the issue description, is to pin scikit-learn until we figure out what is happening so we can release BrainIAK v0.10.
We can even give ourselves a deadline, let's say version BrainIAK v0.11, and remove the test if we cannot make it work by then.
Activity
CameronTEllis commentedon Dec 8, 2019
Is anyone working on this? I can take a crack at it if no one else is
mihaic commentedon Dec 9, 2019
mihaic commentedon Dec 10, 2019
@CameronTEllis, I think it is better to contine the conversation from PR #437 here. Your explanation makes sense; it is likely the length difference is caused by a change in accuracy. Nevertheless, all but one of the accuracy values are the same. Therefore, I think it should be possible to preserve the accuracy value comparison in the test.
CameronTEllis commentedon Dec 10, 2019
Indeed the difference between the results are similar but that is incidental as far as I can tell. Again, I am not sure why the value comparison is the appropriate test because if so it sets an extreme standard that should affect all of our tests. If it is necessary to include this test then we can put in an if statement that deals with this different computation. Other than that I am unsure how to proceed
mihaic commentedon Dec 10, 2019
You are right, we have no standard for tests requiring such comparisons. However, I see no harm keeping this existing test, as long as it is working, and it seems close to working to me.
My proposal, as written in the issue description, is to pin scikit-learn until we figure out what is happening so we can release BrainIAK v0.10.
We can even give ourselves a deadline, let's say version BrainIAK v0.11, and remove the test if we cannot make it work by then.
@yidawang, perhaps you have more insight.
CameronTEllis commentedon Dec 10, 2019
That sounds reasonable then
dev: Pin scikit-learn <0.22 to fix fcma test
dev: Pin scikit-learn <0.22 to fix fcma test
dev: Pin scikit-learn <0.22 to fix fcma test (#450)