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

PUBDEV-8691 Single Decision Tree implementation #6447

Merged
merged 133 commits into from
Jun 12, 2023
Merged

Conversation

syzonyuliia
Copy link
Contributor

@syzonyuliia syzonyuliia commented Dec 9, 2022

Copy link
Collaborator

@valenad1 valenad1 left a comment

Choose a reason for hiding this comment

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

There is couple of stuff that should be removed or improved before it goes public.

IMHO when we remove the stuff that I mention, we can have it in the codebase and polish it in different PR. There is no public documentation - only power user can try it with

from h2o.estimatorsimport H2ODecisionTreeEstimator

since there is no public documentation yet.

I would also consider to put it Expeprimental for the first implementation as SVM is.

  @Override
  public BuilderVisibility builderVisibility() { return BuilderVisibility.Experimental; }

TODOs I see for the differents PRs:

  • scalability benchmarks
  • Refactor DT API - the purpose of DT is to have nicer API than DTree
  • Documentation in sphinx
  • jupyter demo

And please rebase on current master, there are some conflict mentioned by github.

// compute score for given point
CompressedDT tree = DKV.getGet(_output._treeKey);
DTPrediction prediction = tree.predictRowStartingFromNode(data, 0, "");
// System.out.println(prediction.ruleExplanation);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove sout

@mn-mikke mn-mikke merged commit 992b938 into master Jun 12, 2023
@mn-mikke mn-mikke deleted the yuliia-PUBDEV-8691-sdt branch June 12, 2023 08:37
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

Successfully merging this pull request may close these issues.

7 participants