Skip to content

Fix Div integer semantics to use trunc division#7585

Merged
justinchuby merged 8 commits intoonnx:mainfrom
MagellaX:fix/div-floor-7570
Jan 22, 2026
Merged

Fix Div integer semantics to use trunc division#7585
justinchuby merged 8 commits intoonnx:mainfrom
MagellaX:fix/div-floor-7570

Conversation

@MagellaX
Copy link
Copy Markdown
Contributor

@MagellaX MagellaX commented Jan 19, 2026 •
edited by justinchuby
Loading

  • Use trunc division for integer Div in the reference implementation.
  • Add regression tests and document integer Div semantics.

Fixes #7570

@MagellaX
Fix integer division to use floor semantics
c5bae4f 
Signed-off-by: yash solanki <alphacr792@gmail.com>
@MagellaX MagellaX requested review from a team as code owners January 19, 2026 08:54
@github-project-automation github-project-automation Bot moved this to In progress in PR Tracker Jan 19, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 19, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.24%. Comparing base (a78c9c7) to head (2ed9ca6).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnx/backend/test/case/node/div.py 0.00% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7585      +/-   ##
==========================================
- Coverage   55.25%   55.24%   -0.01%     
==========================================
  Files         515      515              
  Lines       32349    32356       +7     
  Branches     2894     2894              
==========================================
+ Hits        17873    17876       +3     
- Misses      13686    13690       +4     
  Partials      790      790

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@titaiwangms titaiwangms requested a review from Copilot January 20, 2026 20:34
Copilot AI reviewed Jan 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the integer division semantics in ONNX to use floor division (rounding toward negative infinity) instead of potentially using truncation division. The change ensures consistency with NumPy's floor division operator and adds comprehensive documentation and tests.

Changes:

  • Updated reference implementation to use np.floor_divide() for integer division
  • Added regression tests verifying floor division behavior with negative numbers
  • Documented integer division semantics across all opset versions (1, 6, 7, 13, 14)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
onnx/reference/ops/op_div.py Changed integer division implementation to explicitly use np.floor_divide()
onnx/test/reference_evaluator_test.py Added test case verifying floor division behavior with negative integers
onnx/backend/test/case/node/div.py Added test case with specific negative number combinations to verify floor division
onnx/defs/math/defs.cc Added documentation for integer floor division in opset 14 schema
onnx/defs/math/old.cc Added documentation for integer floor division in opsets 6, 7, and 13 schemas
docs/Operators.md Updated generated documentation to include floor division description

@MagellaX
Copy link
Copy Markdown
Contributor Author

MagellaX commented Jan 21, 2026

any thoughts? @titaiwangms
i think it looks good to merge!

titaiwangms
titaiwangms reviewed Jan 21, 2026
View reviewed changes
Comment thread onnx/reference/ops/op_div.py Outdated
@MagellaX
Align integer Div with truncating semantics
7386716 
Signed-off-by: yash solanki <alphacr792@gmail.com>
@titaiwangms titaiwangms added topic: bug fix topic: operator Issues related to ONNX operators labels Jan 21, 2026
justinchuby
justinchuby reviewed Jan 21, 2026
View reviewed changes
Comment thread onnx/defs/math/defs.cc Outdated
justinchuby
justinchuby reviewed Jan 21, 2026
View reviewed changes
Comment thread onnx/defs/math/old.cc Outdated
justinchuby
justinchuby reviewed Jan 21, 2026
View reviewed changes
Comment thread onnx/reference/ops/op_div.py
@justinchuby justinchuby modified the milestones: 1.20.1, 1.21 Jan 21, 2026
@justinchuby
Copy link
Copy Markdown
Member

justinchuby commented Jan 21, 2026

cc @fdwr @yuanyao-nv

@justinchuby
Copy link
Copy Markdown
Member

justinchuby commented Jan 21, 2026

@MagellaX Please also fix lint, thanks

MagellaX and others added 2 commits January 22, 2026 02:30
@MagellaX
Merge branch 'main' into fix/div-floor-7570
a4d47f9
@MagellaX
Inline integer Div doc notes
07c20b4 
Signed-off-by: yash solanki <alphacr792@gmail.com>
@fdwr
Copy link
Copy Markdown
Contributor

fdwr commented Jan 22, 2026 •
edited
Loading

Use floor division for integer Div in the reference implementation.

Err, I don't think we should change Div's behavior for integers which existed for many many years as truncation toward zero (you'll break lots of EPs). Looking at spec commit 49e4798, that was a breaking change o_o (note this will also impact WebNN which uses truncating division and calls via Chromium into ORT). If we want a DivFloor with flooring behavior, then let's add a new operator (totally supportive of the intent of having a mode to round toward negative infinity).

@MagellaX
Copy link
Copy Markdown
Contributor Author

MagellaX commented Jan 22, 2026

Use floor division for integer Div in the reference implementation.

Err, I don't think we should change Div's behavior for integers which existed for many many years as truncation toward zero (you'll break lots of EPs). Looking at spec commit 49e4798, that was a breaking change o_o (note this will also impact WebNN which uses truncating division and calls via Chromium into ORT). If we want a DivFloor with flooring behavior, then let's add a new operator (totally supportive of the intent of having a mode to round toward negative infinity).

Thanks for the clarification. I’ve kept integer Div as truncation‑toward‑zero (no behavioral change) and updated the docs accordingly. The reference implementation now uses an integer‑only truncation path (no np.trunc(x / y) to avoid float precision loss). If a DivFloor op is desired, I’m happy to explore that separately. just lmk!

@MagellaX MagellaX requested a review from titaiwangms January 22, 2026 19:10
@MagellaX
Drop reference evaluator Div truncation test
a5031aa 
Signed-off-by: yash solanki <alphacr792@gmail.com>
@justinchuby justinchuby added the auto update doc Generate md/proto files automatically using the CI pipeline label Jan 22, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in PR Tracker Jan 22, 2026
@justinchuby justinchuby reopened this Jan 22, 2026
@github-project-automation github-project-automation Bot moved this from Done to In progress in PR Tracker Jan 22, 2026
justinchuby and others added 3 commits January 22, 2026 19:35
@justinchuby @github-actions
CI:apply auto updated documentation/backend test data
19f7e56 
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@MagellaX
Update generated docs for Div integer semantics
2d19a2e 
Signed-off-by: yash solanki <alphacr792@gmail.com>
@MagellaX @github-actions
CI:apply auto updated documentation/backend test data
2ed9ca6 
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@justinchuby justinchuby changed the title Fix Div integer semantics to use floor division Fix Div integer semantics to use trunc division Jan 22, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Jan 22, 2026
@justinchuby justinchuby removed the auto update doc Generate md/proto files automatically using the CI pipeline label Jan 22, 2026
@justinchuby justinchuby reopened this Jan 22, 2026
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Jan 22, 2026
@github-project-automation github-project-automation Bot moved this from Done to In progress in PR Tracker Jan 22, 2026
@justinchuby
Copy link
Copy Markdown
Member

justinchuby commented Jan 22, 2026

Thank you!

@justinchuby justinchuby enabled auto-merge (squash) January 22, 2026 20:21
@justinchuby justinchuby disabled auto-merge January 22, 2026 20:21
@justinchuby justinchuby enabled auto-merge (squash) January 22, 2026 20:21
@justinchuby justinchuby merged commit 2eb3d99 into onnx:main Jan 22, 2026
46 of 47 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Done in PR Tracker Jan 22, 2026
@HudsonGraeme HudsonGraeme mentioned this pull request Jan 22, 2026
Closed
@BrewTestBot BrewTestBot mentioned this pull request Mar 27, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@justinchuby justinchuby justinchuby approved these changes

@titaiwangms titaiwangms Awaiting requested review from titaiwangms

Assignees

No one assigned

Labels

topic: bug fix topic: operator Issues related to ONNX operators

Projects

PR Tracker
Status: Done

Milestone

1.21

Development

Successfully merging this pull request may close these issues.

Integer division inconsistency

5 participants

@MagellaX @justinchuby @fdwr @titaiwangms