[MRG] ENH: multi-output support for BaggingRegressor #8547

dohmatob · 2017-03-06T22:29:02Z

Closes #8993, #3449, #4848. This minimalistic patch adds multi-output support to BaggingRegressor.

raghavrv

This is nice... Also could you add a whatsnew...

raghavrv · 2017-03-07T11:15:35Z

sklearn/ensemble/tests/test_bagging.py

+                                                        boston.target,
+                                                        random_state=rng)
+
+    reg1 = BaggingRegressor(base_estimator=Ridge(), n_estimators=1,


Can you test for n_estimators>1?

Ya you're absolutely right. n_estimators=1 makes no sense within a test called "bagging" :). Fixed.

jnothman · 2017-03-07T12:19:31Z

sklearn/ensemble/bagging.py

@@ -281,7 +283,7 @@ def _fit(self, X, y, max_samples=None, max_depth=None, sample_weight=None):
        random_state = check_random_state(self.random_state)

        # Convert data
-        X, y = check_X_y(X, y, ['csr', 'csc'])
+        X, y = check_X_y(X, y, ['csr', 'csc'], multi_output=self.multi_output)


I don't get why we can't just use multi_output=True

If multioutput is false, we don't want y to be having more than one columns no?

Ya, @johannah I don't see why you you'd rather not have the multi_output=True in that func call.

After second thoughts, i think @jnothman has a point. We can make BaggingRegressor support multi-output targets by simply

Forcing multi_output=True in the call check_X_y(...)

Skipping the _validate_y(...) thing. I guess the question should we really be skipping this ?

jnothman · 2017-03-07T21:30:37Z

check_X_y is clearly documented to say that this parameter controls whether multioutput is allowed, not required. 1d y will remain 1d. The only difference might be for column vectors On 8 Mar 2017 7:24 am, "(Venkat) Raghav (Rajagopalan)" < notifications@github.com> wrote: *@raghavrv* commented on this pull request. ------------------------------ In sklearn/ensemble/bagging.py <#8547 (comment)> :

@@ -281,7 +283,7 @@ def _fit(self, X, y, max_samples=None, max_depth=None, sample_weight=None):

random_state = check_random_state(self.random_state) # Convert data - X, y = check_X_y(X, y, ['csr', 'csc']) + X, y = check_X_y(X, y, ['csr', 'csc'], multi_output=self.multi_output) If multioutput is false, we don't want y to be having more than one columns no? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8547 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6zsBd0RH2bQeg9LUXXoW6aU-S2V3ks5rjbzvgaJpZM4MUyGz> .

jnothman · 2017-06-06T11:10:33Z

I don't think we need a classifier parameter to make this work, do we?

dohmatob · 2017-06-08T15:06:29Z

@johannah: I don't get your question, please could you be more precise.

jnothman · 2017-06-08T23:37:15Z

we can skip out change whatever is making inappropriate constraints

…

On 9 Jun 2017 1:48 am, "DOHMATOB Elvis" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sklearn/ensemble/bagging.py <#8547 (comment)> : > @@ -281,7 +283,7 @@ def _fit(self, X, y, max_samples=None, max_depth=None, sample_weight=None): random_state = check_random_state(self.random_state) # Convert data - X, y = check_X_y(X, y, ['csr', 'csc']) + X, y = check_X_y(X, y, ['csr', 'csc'], multi_output=self.multi_output) After second thoughs, i think @jnothman <https://github.com/jnothman> has a point. We can make BaggingRegressor support multi-output targets by simply - Forcing multi_output=True in the call check_X_y(...) - Skipping the _validate_y(...) thing. I guess the question should we really be skipping this ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8547 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz67SK23dZm6bdKupvWNWG2XRSQbqIks5sCBfhgaJpZM4MUyGz> .

jnothman · 2017-06-15T08:05:01Z

doc/whats_new.rst

@@ -843,6 +843,9 @@ Trees and ensembles
     :class:`ensemble.VotingClassifier` to fit underlying estimators in parallel.
     :issue:`5805` by :user:`Ibraim Ganiev <olologin>`.

+   - :class:`ensemble.BaggingRegressor` now supports multi-output targets.


This needs to move to the v0.19 section.

jnothman · 2017-06-15T08:06:30Z

sklearn/ensemble/bagging.py

@@ -390,8 +390,9 @@ def _set_oob_score(self, X, y):
        """Calculate out of bag predictions and score."""

    def _validate_y(self, y):
-        # Default implementation
-        return column_or_1d(y, warn=True)


So there should still be a check_array call here. And if not, we should delete this method.

Indeed, this might be right, since the validation is done by the downstream estimator.

jnothman

We've had several contributions of multi-output support for bagging. I think keeping it minimal here (not trying to do classification at the same time) is a good idea.

jnothman · 2017-06-19T02:06:47Z

sklearn/ensemble/bagging.py

@@ -390,8 +390,9 @@ def _set_oob_score(self, X, y):
        """Calculate out of bag predictions and score."""

    def _validate_y(self, y):
-        # Default implementation
-        return column_or_1d(y, warn=True)


Indeed, this might be right, since the validation is done by the downstream estimator.

jnothman · 2017-06-19T02:09:13Z

sklearn/ensemble/tests/test_bagging.py

+    reg1.fit(X_train, y_train)
+    for n_targets in [1, 2]:
+        y_train_ = np.ndarray((len(y_train), n_targets))
+        y_train_.T[:] = y_train.copy()


I'm finding this hard to understand.

amueller · 2018-05-22T18:52:16Z

What's the status on this?

stefanmeili · 2018-10-15T15:49:55Z

I'd also really like to see this implemented.

jlnzhou · 2021-03-02T12:07:40Z

Hi!
This PR is stalled, can I try to work on this issue if it is suitable for a new contributor?

adrinjalali · 2024-04-17T11:32:13Z

Closing as implemented in #9707

dohmatob force-pushed the general-bagging branch 2 times, most recently from f8f4c27 to a178084 Compare March 7, 2017 07:28

raghavrv suggested changes Mar 7, 2017

View reviewed changes

jnothman reviewed Mar 7, 2017

View reviewed changes

raghavrv changed the title ~~ENH: multi-output support for BaggingRegressor~~ [MRG] ENH: multi-output support for BaggingRegressor Mar 7, 2017

raghavrv added Enhancement Waiting for Reviewer labels Mar 7, 2017

raghavrv removed the Waiting for Reviewer label Mar 13, 2017

dohmatob force-pushed the general-bagging branch 2 times, most recently from 82973cc to 13ef011 Compare March 24, 2017 14:36

dohmatob force-pushed the general-bagging branch 3 times, most recently from 86e1d16 to 2bbf3ab Compare June 6, 2017 10:25

dohmatob force-pushed the general-bagging branch from 2bbf3ab to 0bceacb Compare June 8, 2017 15:03

dohmatob force-pushed the general-bagging branch 6 times, most recently from 8e68e4e to 168eaf3 Compare June 8, 2017 18:27

dohmatob force-pushed the general-bagging branch 2 times, most recently from e3acfa4 to 7b9b39f Compare June 10, 2017 11:39

jnothman reviewed Jun 15, 2017

View reviewed changes

jnothman reviewed Jun 19, 2017

View reviewed changes

ENH: multi-output support for BaggingRegressor

893eab9

dohmatob force-pushed the general-bagging branch from 7b9b39f to 893eab9 Compare February 2, 2018 01:08

jnothman added the Stalled label Oct 16, 2018

amueller added the Needs work label Aug 6, 2019

github-actions bot added module:ensemble module:utils labels Mar 2, 2020

cmarmo added the help wanted label Sep 21, 2020

cmarmo mentioned this pull request Sep 21, 2020

Issue-3449: Add multioutput support to bagging #3798

Closed

Base automatically changed from master to main January 22, 2021 10:49

cmarmo mentioned this pull request May 2, 2022

[MRG+1] Multioutput bagging #4848

Closed

hmasdev mentioned this pull request Nov 2, 2023

[MRG] ENH: Make StackingRegressor support Multioutput #27704

Open

adrinjalali closed this Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] ENH: multi-output support for BaggingRegressor #8547

[MRG] ENH: multi-output support for BaggingRegressor #8547

dohmatob commented Mar 6, 2017 •

edited by jnothman

Loading

raghavrv left a comment

raghavrv Mar 7, 2017

dohmatob Jun 6, 2017 •

edited

Loading

jnothman Mar 7, 2017

raghavrv Mar 7, 2017

dohmatob Jun 6, 2017

dohmatob Jun 8, 2017 •

edited

Loading

jnothman commented Mar 7, 2017 via email

jnothman commented Jun 6, 2017

dohmatob commented Jun 8, 2017

jnothman commented Jun 8, 2017 via email

jnothman Jun 15, 2017

jnothman Jun 15, 2017

jnothman Jun 19, 2017

jnothman left a comment

jnothman Jun 19, 2017

jnothman Jun 19, 2017

amueller commented May 22, 2018

stefanmeili commented Oct 15, 2018

jlnzhou commented Mar 2, 2021

adrinjalali commented Apr 17, 2024

[MRG] ENH: multi-output support for BaggingRegressor #8547

[MRG] ENH: multi-output support for BaggingRegressor #8547

Conversation

dohmatob commented Mar 6, 2017 • edited by jnothman Loading

raghavrv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dohmatob Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dohmatob Jun 8, 2017 • edited Loading

Choose a reason for hiding this comment

jnothman commented Mar 7, 2017 via email

jnothman commented Jun 6, 2017

dohmatob commented Jun 8, 2017

jnothman commented Jun 8, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amueller commented May 22, 2018

stefanmeili commented Oct 15, 2018

jlnzhou commented Mar 2, 2021

adrinjalali commented Apr 17, 2024

dohmatob commented Mar 6, 2017 •

edited by jnothman

Loading

dohmatob Jun 6, 2017 •

edited

Loading

dohmatob Jun 8, 2017 •

edited

Loading