Skip to content

Refactor TreeEnsemble weight loop: validate array sizes once before iteration - #27679

Merged
xadupre merged 3 commits into
xadupre/tree110413from
copilot/sub-pr-27677-again
Mar 26, 2026
Merged

Refactor TreeEnsemble weight loop: validate array sizes once before iteration#27679
xadupre merged 3 commits into
xadupre/tree110413from
copilot/sub-pr-27677-again

Conversation

Copilot AI commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Description

Addresses review feedback on #27677. The three per-iteration ORT_ENFORCE bounds checks inside the weight assignment loop in tree_ensemble_common.h have been moved to a single pre-loop validation block.

Before: on every iteration, the loop checked i < target_class_ids.size(), i < target_class_weights.size(), and i < target_class_weights_as_tensor.size().

After: since indices is built from target_class_nodeids with consecutive indices [0, limit), i = indices[indi].second is always in range — so size relationships are validated once upfront with == limit equality checks:

limit = attributes.target_class_nodeids.size();
ORT_ENFORCE(attributes.target_class_ids.size() == limit,
            "target_class_ids size (", attributes.target_class_ids.size(),
            ") must match target_class_nodeids size (", limit, ")");
ORT_ENFORCE(attributes.target_class_weights_as_tensor.empty() ||
                attributes.target_class_weights_as_tensor.size() == limit, ...);
ORT_ENFORCE(!attributes.target_class_weights_as_tensor.empty() ||
                attributes.target_class_weights.size() == limit, ...);
for (indi = 0; indi < limit; ++indi) {
    // no per-element bounds checks needed
}

Motivation and Context

The per-iteration checks made the control flow harder to reason about and repeated redundant work on every loop iteration. Validating the authoritative array lengths once — before the loop — makes it clear which arrays must be parallel and eliminates repeated checks.

Original PR: #27677


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on ORT_ENFORCE additions for TreeEnsemble operators Refactor TreeEnsemble weight loop: validate array sizes once before iteration Mar 16, 2026
Copilot AI requested a review from xadupre March 16, 2026 16:28
@xadupre
xadupre marked this pull request as ready for review March 26, 2026 16:22
@xadupre
xadupre merged commit fb31dde into xadupre/tree110413 Mar 26, 2026
10 checks passed
@xadupre
xadupre deleted the copilot/sub-pr-27677-again branch March 26, 2026 16:22
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.

2 participants