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.

Sign up for GitHub

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

Jump to bottom

Implementation of TreeEnsemble ai.onnx.ml==5 #22333

Merged
merged 49 commits into from
Nov 22, 2024
Merged

Implementation of TreeEnsemble ai.onnx.ml==5 #22333

merged 49 commits into from
Nov 22, 2024

Conversation

xadupre
Copy link
Member

@xadupre xadupre commented Oct 7, 2024

Description

Merges PR #21851, #21222.

Implements TreeEnsemble from ai.onnx.ml==5 (CPU).

bili2002 and others added 30 commits July 1, 2024 14:58
@bili2002
implement solution
01104aa
@bili2002
Change reinterpret_cast to bit_cast as it is safer
6bd62ad
@bili2002
not use std::bit_cast as it is not supported in c++20
249a327
@bili2002
fix
b0e8c80
@bili2002
Basic solution without setmembership
be7b35a
@bili2002
Add tests, debug and multitree support
d002ea1
@bili2002
make more readable
c12554d
@bili2002
Add setmembership support and a test
29e4581
@bili2002
Add const to some parameters
202f3f6
@bili2002
Add leaf only support; and small test fix
5f59c52
@bili2002
Small changes
a373f29
@bili2002 @cbourjau
Update onnxruntime/core/providers/cpu/ml/tree_ensemble.h
2cf3d23 
Co-authored-by: Christian Bourjau <[email protected]>
@bili2002
docs change
167d0b1
@bili2002
Renaming
9211428
@bili2002
Formatting
b2ba77d
@bili2002
small changes
d54b084
@bili2002
Update docs
55bd1a2
@bili2002
formatting
0092896
@bili2002
formatting
99adf5b
@bili2002
Fix windows error
1b15a1d
@bili2002
Fix windows error
11c3d38
@bili2002
Fix windows errors
09ec286
@bili2002
fix windows errors
c135ee0
@bili2002
Merge branch 'main' into tree-ensemble-v5
175cb1a
@bili2002
Fix buffer overflow
b91d7bc
@xadupre
Merge branch 'tree-ensemble-v5' of https://github.com/bili2002/onnxru…
8d0bc46 
…ntime into tree5
@xadupre
fix merge conflict
125eedc
@xadupre
fix build
66d6b73
@xadupre
add missing namespace
7823d74
@xadupre
restore cc
e4b825a
@xadupre xadupre changed the title [WIP] Implementation of TreeEnsemble ai.onnx.ml==5 Implementation of TreeEnsemble ai.onnx.ml==5 Oct 11, 2024
yuslepukhin
yuslepukhin reviewed Oct 11, 2024
View reviewed changes
Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

Lots of memory allocations. I had to rewerite a couple kernels in the past due to high allocation contentions that actually affected performance. Is it possible to use InlinedVector at least in some instances?

Using spans in function signagures would enable taking both kinds of vectors as inputs.

onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Outdated Show resolved Hide resolved
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Show resolved Hide resolved
@bili2002
Copy link
Contributor

I don't have permission to push in this branch, can you give me permission?

@bili2002 bili2002 mentioned this pull request Oct 21, 2024
Closed
@cbourjau
Copy link
Contributor

cbourjau commented Nov 4, 2024

Is there anything we can do to push this forward, @xadupre ?

@xadupre
Copy link
Member Author

xadupre commented Nov 4, 2024

Is there anything we can do to push this forward, @xadupre ?

I did a first refactoring today to avoid very long function signatures. There are a couple of other improvments I still need to make.

This was referenced Nov 5, 2024
Closed
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 19, 2024
View reviewed changes
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Dismissed Show dismissed Hide dismissed
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Dismissed Show dismissed Hide dismissed
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Dismissed Show dismissed Hide dismissed
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h
root = SetMembershipCheck(val, root->value_or_unique_weight) ? root->truenode_or_weight.ptr : root + 1;
}
}
case NODE_MODE_ORT::LEAF:

Check warning

Code scanning / PREfast

Unannotated fallthrough between switch labels (es.78). Warning

Unannotated fallthrough between switch labels (es.78).
onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Dismissed Show dismissed Hide dismissed
@xadupre xadupre merged commit a2ba3cb into microsoft:main Nov 22, 2024
92 checks passed
guschmue pushed a commit that referenced this pull request Dec 2, 2024
@xadupre @bili2002
@cbourjau @guschmue
Implementation of TreeEnsemble ai.onnx.ml==5 (#22333)
486fb01 
### Description
Merges PR #21851, #21222.

Implements TreeEnsemble from ai.onnx.ml==5 (CPU).

---------

Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Christian Bourjau <[email protected]>
ankitm3k pushed a commit to intel/onnxruntime that referenced this pull request Dec 11, 2024
@xadupre @bili2002
@cbourjau @ankitm3k
Implementation of TreeEnsemble ai.onnx.ml==5 (microsoft#22333)
23f88f3 
### Description
Merges PR microsoft#21851, microsoft#21222.

Implements TreeEnsemble from ai.onnx.ml==5 (CPU).

---------

Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Christian Bourjau <[email protected]>
ankitm3k pushed a commit to intel/onnxruntime that referenced this pull request Dec 11, 2024
@xadupre @bili2002
@cbourjau @ankitm3k
Implementation of TreeEnsemble ai.onnx.ml==5 (microsoft#22333)
0346a06 
### Description
Merges PR microsoft#21851, microsoft#21222.

Implements TreeEnsemble from ai.onnx.ml==5 (CPU).

---------

Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Bilyana Indzheva <[email protected]>
Co-authored-by: Christian Bourjau <[email protected]>
@khoover khoover mentioned this pull request Jan 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuslepukhin yuslepukhin yuslepukhin approved these changes

@skottmckay skottmckay Awaiting requested review from skottmckay

@bili2002 bili2002 Awaiting requested review from bili2002

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xadupre @bili2002 @cbourjau @yuslepukhin