Skip to content

Skip constant folding for non-deterministic Bernoulli - #3072

Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 2 commits into
microsoft:mainfrom
MohammedAlkindi:fix/fold-skip-bernoulli
Open

Mohammed Alkindi (MohammedAlkindi) wants to merge 2 commits into
microsoft:mainfrom
MohammedAlkindi:fix/fold-skip-bernoulli

Conversation

@MohammedAlkindi

Copy link
Copy Markdown
Contributor

Bernoulli was missing from _NON_DETERMINISTIC_OPS, so constant folding evaluated it once during optimization and replaced it with a fixed tensor. A Bernoulli on a constant probability input, such as a dropout-style mask, then returns the same sample on every run. It now stays in the graph, like the RandomUniform, RandomNormal and Multinomial ops already in the set.

test_bernoulli_is_not_folded fails before ([] != ['Bernoulli']) and passes after. onnxscript/optimizer: 75 passed. ruff check and ruff format --check are clean.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix follows existing handling and includes an appropriate regression test, with no unresolved findings.

0 open findings

What changed in this PR

Prevents constant folding from freezing Bernoulli samples, preserving randomness across model runs.

Changes:

  • Adds Bernoulli to the existing non-deterministic operator exclusion set.
  • Adds a regression test verifying it remains in the graph with constant input.
File Description
onnxscript/​optimizer/​_constant_folding.py Excludes Bernoulli from constant folding.
onnxscript/​optimizer/​_constant_folding_test.py Tests preservation of Bernoulli with constant probabilities.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.02%. Comparing base (1cd0878) to head (41c5e65).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3072      +/-   ##
==========================================
+ Coverage   73.01%   73.02%   +0.01%     
==========================================
  Files         267      267              
  Lines       32641    32647       +6     
  Branches     3101     3101              
==========================================
+ Hits        23832    23841       +9     
+ Misses       7767     7765       -2     
+ Partials     1042     1041       -1     

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

Justin Chu (justinchuby) pushed a commit that referenced this pull request Oct 28, 2026
Constant folding treats a training-mode `Dropout` on a constant input
like any other op: it runs the reference implementation once during
optimization and stores the sampled result as an initializer. The
optimized model then drops the same elements on every run instead of
drawing a new mask.

This is reachable from `torch.onnx.export`, which runs `optimize()` by
default. For example, `F.dropout(self.weight, 0.5, training=True)` on a
small parameter exports as `Dropout` followed by `Add`, and after
optimization only the `Add` is left, reading a fixed tensor. Two exports
of the same module bake in two different masks.

The fix adds `Dropout` to `_NON_DETERMINISTIC_OPS`. The `Dropout`
partial evaluator still runs before that check, so inference-mode and
`ratio=0` dropouts are still replaced by `Identity` as before; only the
random case stays in the graph.

`test_dropout_in_training_mode_on_constant_input_is_not_folded` fails
before (`['Add'] != ['Dropout', 'Add']`) and passes after.
`onnxscript/optimizer`: 81 passed; `onnxscript/rewriter`: 581 passed, 3
skipped.

This is the same kind of freeze as `Bernoulli` in #3072, kept separate
because `Dropout` is only random in training mode. Happy to fold it into
#3072 or rebase after it lands if you would rather handle it there.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants