Thanks for making this PR!

I think a better place to perform this check is in Base.fit() in sklearn/neureal_network/_multilayer_perceptron.py. There you can use self.validation_fraction instead of having to hard-code 0.1. The check should only happen if early_stopping is being used.

Thanks @betatim , great tip. Will get back on this.

Thanks for making this PR!

I think a better place to perform this check is in Base.fit() in sklearn/neureal_network/_multilayer_perceptron.py. There you can use self.validation_fraction instead of having to hard-code 0.1. The check should only happen if early_stopping is being used.

Hi @betatim , I updated the Base.fit() in sklearn/neureal_network/_multilayer_perceptron.py with an if loop to check self.validation_fraction as suggested by you. You should be able to see it now.

Of course, this implies that for any training input where len(X) is less than 11, a ValueError will be raised. This is the intention or? Could there be some angles of the bigger picture that I might be missing here?

Thank you.

Apologies, I didn't read the code carefully enough, fit() isn't a good place. I've now read the code and think adding

if X_val.shape[0] < 2:
    raise ValueError("The validation set is too small. Increase the validation_fraction or the size of your dataset.")

after line 581 (where X_val is defined) in _multilayer_perceptron.py is the thing to do.

This way we can directly check that X_val contains enough samples (more than two).

I've also made a suggestion for the error message.

Thanks for the swift feedback @betatim

X_val is unfortunately not used in fit or _fit function in the BaseMultilayerPerceptron. Also, fit is not checking early_stopping criteria. Could this be a possibility in _fit (at the cost of repetition though):

early_stopping = self.early_stopping
X, X_val, y, y_val = train_test_split(
                X,
                y,
                random_state=self._random_state,
                test_size=self.validation_fraction,
                stratify=stratify,
            )
if (early_stopping and (X_val.shape[0] < 2)):
        raise ValueError("The validation set is too small. Increase the validation_fraction or the size of your dataset.")

X_val is created in _fit_stochastic(), so the check that it contains enough samples needs to be there as well. As far as I can see _fit_stochastic is the only place where a validation dataset is created, because it is the only place where early_stopping = True changes the behaviour of the estimator.

I see. Then, if I understood to somewhat correct degree, here is what could work:

1. early stopping matters only in _fit_stochastic solver, so its sort of independent from checking the input dataset size or validation_fraction in general.
2. But _fit_stochastic is called by _fit in BaseMultilayerPerceptron.
3. Therefore, by putting the if check right after self._validate_input in _fit function will likely solve the whole issue and we only do it once. One does not need to put an extra check for _fit_stochastic with early_stopping as its already been taken care by _fit due to independence.

You should already see it in the PR update. Please let me know if it makes sense or if I am missing something in the issue.

Hi @betatim , wonder if you had a chance to look at the solution above? Let me know if it addresses the issue. Thanks.

HI, unfortunately we misunderstood each other.

X_val is created in _fit_stochastic(), so the check that it contains enough samples needs to be there as well.

Ok, so the recent commit contains the following two additions:

1. added an if check in _fit function in the BaseMultilayerPerceptron class

if (self.validation_fraction * X.shape[0] < 2):
    raise ValueError("The validation set is too small. Increase the validation_fraction or the size of your dataset.")

2. added an additional if check within the _fit_stochastic function:

if X_val.shape[0] < 2:
    raise ValueError("The validation set is too small. Increase the validation_fraction or the size of your dataset.")

I suspect the reason to add this additional check in _fit_stochastic is because _fit_stochastic is called upon in other places in the code than just from _fit, or?

I would remove your first addition and only check in _fit_stochastic. The reason being that there you can test what you really want to test: does the validation dataset contain enough samples? In _fit you'd have to replicate the exact behaviour of how train_test_split() (which is used to create the validation set) computes how many samples will be in the validation set. This is why I think checking in _fit_stochastic is the better thing to do.

I see, got it @betatim - thanks for the clarification. Done - now, we only have the check in _fit_stochastic.
Would you know what the next steps should be with this issue?
Thanks.

Two more things are needed:

1. a non-regression test to make sure the bug doesn't come back. Maybe based on the example reported in AttributeError: 'MLPRegressor' object has no attribute '_best_coefs' #24713
   • the test should fail on main and pass with the changes from this PR
2. an entry in the "what's new" change log that reports that this bug has been fixed

@betatim - sorry for a bit of delay in coming back to it. I have added a test (last function) in the sklearn/neural_network/tests/test_mlp.py, which now checks the minimum input sample size.

It passes the test when pytest sklearn/neural_network/tests/test_mlp.py but when I just run pytest for all the tests, the following two (seemingly unrelated) errors show up:

1. E AttributeError: module 'sklearn.metrics._pairwise_distances_reduction._base' has no attribute 'BaseDistanceReducer64'
2. E AttributeError: module 'sklearn.metrics._pairwise_distances_reduction._base' has no attribute 'BaseDistanceReducer64'

Could it be due to some deprecated code?
Thank you.

@betatim - could you look at the code (test submitted) above? Any feedback welcome.

The linting tests that check the formatting of the code are failing. Take a look at step 9 of https://scikit-learn.org/stable/developers/contributing.html#how-to-contribute or the section after that for the tools you need to run to have the formatting fixed automatically. The easiest way is to use pre-commit.

I'm not sure why the other two tests fail, they shouldn't be related to your changes. If you fix the linter errors we can see if the problems also happen in the CI itself.

You are right @betatim, shouldn't be from the fix I suggested. Let me have look. Thanks for the feedback.

Done!

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: c3ca73b. Link to the linter CI: here}