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

A bug in the cross_validation function #995

Open
pyaf opened this issue Dec 19, 2018 · 7 comments
Open

A bug in the cross_validation function #995

pyaf opened this issue Dec 19, 2018 · 7 comments

Comments

@pyaf
Copy link

pyaf commented Dec 19, 2018

Hi,

Currently, cross_validation function shuffles data before every step of k fold cross-validation [here]. Which is wrong because of this the test set at each step is not unique (when compared to all the other steps) which gives us the wrong k fold cross validation perfomance score.

Instead, we should shuffle the dataset before performing k fold cross validation (rather than doing the same at each step of k fold cross-validation).

Let me know your thoughts.

@ad71
Copy link
Contributor

ad71 commented Dec 19, 2018

The implementation for the cross-validation algorithm has been up for debate for quite some time now. Refer to this issue and this issue for details. It needs to be re-implemented but we haven't reached a conclusion yet. This will probably be updated in the newer versions of AIMA.
However, I think we can implement a function with a different name for the time-being, that works as we expect cross_validation to.

@pyaf
Copy link
Author

pyaf commented Dec 19, 2018

I see a # TODO: The function cross_validation_wrapper needs to be fixed. (The while loop runs forever!) before cross_validation_wrapper. Is this the issue here? Like what exactly is the debate here?

@ad71
Copy link
Contributor

ad71 commented Dec 19, 2018

The debate here is that the conventional meaning of cross_validation has changed since the time AIMA was first written. The pseudocode in AIMA does different things than what cross_validation in scikit-learn does for example. We are unsure as to what the goal of the cross_validation function in the repo is, to implement the pseudocode from AIMA or the more commonly accepted implementations in modern libraries.

ashishgit7 added a commit to ashishgit7/aima-python that referenced this issue Dec 20, 2018
ashishgit7 added a commit to ashishgit7/aima-python that referenced this issue Dec 20, 2018
@ashishgit7
Copy link
Contributor

@pyaf sir I made a PR related 995 issue now what else we can do here

@Pihu1998
Copy link

Hi! I would like to know whether this issue is resolved.

@ashishgit7
Copy link
Contributor

@pyaf can you review my PR

@pyaf
Copy link
Author

pyaf commented Mar 11, 2019

@hackerashish25 you should ask aima-python mentors for that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants