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

[Track v1.4] New PR branch to serve as submodule for scikit-tree #53

Open
wants to merge 211 commits into
base: main
Choose a base branch
from

Conversation

adam2392
Copy link
Collaborator

Reference Issues/PRs

As of v0.2 for sktree, we have decided we do not need a custom built and released via pypi scikit-learn fork. Instead, we just have to keep an updated fork branch here that maintains the changes under tree/ and ensemble/.

This branch has significantly lower diff and less complexity compared to e.g. #44

What does this implement/fix? Explain your changes.

Any other comments?

@github-actions
Copy link

github-actions bot commented Aug 11, 2023

✔️ Linting Passed

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

Generated for commit: 4fd15fd. Link to the linter CI: here

sklearn/tree/_tree.pyx Outdated Show resolved Hide resolved
@PSSF23

This comment was marked as outdated.

Copy link
Member

@PSSF23 PSSF23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce the error on my local machine with following code:

from sklearn.ensemble import RandomForestClassifier

import numpy as np
from inspect import signature

rnd = np.random.RandomState(0)
n_samples = 30
X = rnd.uniform(size=(n_samples, 3))
y = np.arange(n_samples)

clf_1 = RandomForestClassifier()
clf_1.set_params(random_state=0)

func = getattr(clf_1, "fit", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

func = getattr(clf_1, "score", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

func = getattr(clf_1, "partial_fit", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

The code should replicate most of the check_fit_score_takes_y test, but it runs smoothly every time. I also don't understand why only these 2 CIs failed when all should have the same test library.

  • Linux_Runs pylatest_conda_forge_mkl
  • macOS pylatest_conda_mkl_no_openmp

@adam2392
Copy link
Collaborator Author

I cannot reproduce the error on my local machine with following code:

from sklearn.ensemble import RandomForestClassifier

import numpy as np
from inspect import signature

rnd = np.random.RandomState(0)
n_samples = 30
X = rnd.uniform(size=(n_samples, 3))
y = np.arange(n_samples)

clf_1 = RandomForestClassifier()
clf_1.set_params(random_state=0)

func = getattr(clf_1, "fit", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

func = getattr(clf_1, "score", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

func = getattr(clf_1, "partial_fit", None)
func(X,y)
args = [p.name for p in signature(func).parameters.values()]

The code should replicate most of the check_fit_score_takes_y test, but it runs smoothly every time. I also don't understand why only these 2 CIs failed when all should have the same test library.

  • Linux_Runs pylatest_conda_forge_mkl
  • macOS pylatest_conda_mkl_no_openmp

If you try running the HonestForestClassifier instead, then the error seems to be able to be produced on my computer

@PSSF23
Copy link
Member

PSSF23 commented Aug 25, 2023

@adam2392 Is sktree updated with the current submodule? After the resizing bug is fixed?

@adam2392
Copy link
Collaborator Author

Yeah it should be. The test is commented out tho rn.

@PSSF23

This comment was marked as outdated.

Copy link
Member

@PSSF23 PSSF23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All CIs passed.

Signed-off-by: Adam Li <[email protected]>
@adam2392
Copy link
Collaborator Author

adam2392 commented Oct 6, 2023

TODO: Change

int -> intp_t
double -> float64_t
SIZE_t -> intp_t
DTYPE_t -> float32_t
INT32_t -> int32_t

see: https://github.com/scikit-learn/scikit-learn/pull/27352/files and related PRs

Signed-off-by: Adam Li <[email protected]>
@adam2392
Copy link
Collaborator Author

adam2392 commented Oct 9, 2023

TODO: Change

int -> intp_t double -> float64_t SIZE_t -> intp_t DTYPE_t -> float32_t INT32_t -> int32_t

see: https://github.com/scikit-learn/scikit-learn/pull/27352/files and related PRs

This was accomplished in 9a5d91b

jeremiedbb and others added 30 commits October 3, 2024 09:46
…30019)

Co-authored-by: Lock file bot <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.