Skip to content

Fix flaky GELU test and clippy warnings in candle-onnx#3317

Open
Douglas-Nyberg wants to merge 3 commits intohuggingface:mainfrom
Douglas-Nyberg:fix-gelu-test
Open

Fix flaky GELU test and clippy warnings in candle-onnx#3317
Douglas-Nyberg wants to merge 3 commits intohuggingface:mainfrom
Douglas-Nyberg:fix-gelu-test

Conversation

@Douglas-Nyberg
Copy link

@Douglas-Nyberg Douglas-Nyberg commented Jan 19, 2026

Changes

  • Fix test_gelu_operation in candle-onnx to use approximate float comparison (was failing due to precision differences).
  • Fix clippy warnings in candle-onnx/src/eval.rs.
  • Add clippy allow attributes for pre-existing test issues (excessive float precision) and generated protobuf code.

Why

While running tests locally, I found that test_gelu_operation was flaky due to using exact float equality. Additionally, cargo clippy revealed several issues in eval.rs that needed fixing.

(Note: I originally attempted to add candle-onnx to the workspace/CI, but reverted that infrastructure change per maintainer feedback to focus solely on these code fixes.)

Testing

All 64 candle-onnx tests pass locally. cargo clippy passes.

@Douglas-Nyberg Douglas-Nyberg marked this pull request as draft January 19, 2026 19:31
@Douglas-Nyberg Douglas-Nyberg marked this pull request as ready for review January 19, 2026 19:51
@ivarflakstad
Copy link
Member

Thanks!

I'm happy to merge parts of this PR, but candle-onnx is actually purposefully excluded from workspace / CI.
We'll be making some changes to candle-core (soon ™️ ) which should make it much easier to maintain candle-onnx (and candle-pyo3). At which point we could definitely add it to workspace and CI.

@Douglas-Nyberg Douglas-Nyberg marked this pull request as draft January 20, 2026 20:20
@Douglas-Nyberg
Copy link
Author

Thanks for the feedback!

I've reverted the changes to Cargo.toml and the CI workflow as requested.
This PR now only contains the code-level fixes:

Fix for the flaky GELU test: Switched to approximate comparison to handle floating point differences.
Clippy fixes: Fixed bugs in candle-onnx/src/eval.rs and added an allow attribute for the generated protobuf code.

Let me know if you would like to see something else.

@Douglas-Nyberg Douglas-Nyberg marked this pull request as ready for review January 20, 2026 20:28
@Douglas-Nyberg Douglas-Nyberg changed the title Add candle-onnx to workspace and fix flaky gelu test Fix flaky GELU test and clippy warnings in candle-onnx Jan 20, 2026
Copy link
Member

@ivarflakstad ivarflakstad left a comment

Choose a reason for hiding this comment

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

Lgtm! Thanks :)

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