Skip to content

Vectorize sigmoid op implementation#7556

Merged
justinchuby merged 7 commits intomainfrom
justinchu/vectorize
Jan 6, 2026
Merged

Vectorize sigmoid op implementation#7556
justinchuby merged 7 commits intomainfrom
justinchu/vectorize

Conversation

@justinchuby
Copy link
Copy Markdown
Member

@justinchuby justinchuby commented Jan 6, 2026

Make it faster.

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby requested a review from a team as a code owner January 6, 2026 18:27
@github-project-automation github-project-automation Bot moved this to In progress in PR Tracker Jan 6, 2026
@justinchuby justinchuby changed the title Vectorize sigmoid op implementations Vectorize sigmoid op implementation Jan 6, 2026
@justinchuby justinchuby added this to the 1.21 milestone Jan 6, 2026
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
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 vectorizes the sigmoid operation to improve performance by removing the inefficient np.vectorize wrapper and implementing direct numpy array operations. The sigmoid function is commonly used and this change aims to make it faster.

Key Changes

  • Refactored the sigmoid function to operate directly on numpy arrays instead of scalar values
  • Removed np.vectorize usage from the Sigmoid class, which was a performance bottleneck
  • Removed the special case handling for scalar inputs (len(X.shape) == 0) as it's no longer needed
Comments suppressed due to low confidence (1)

onnx/reference/ops/op_sigmoid.py:25

  • The dtype of the result is not being preserved. The result should be cast to match the input dtype. All similar operations in the codebase (e.g., op_exp.py, op_log.py, op_relu.py) use .astype(x.dtype) to preserve the input dtype. The return statement should ensure the result matches the input dtype.

Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby requested a review from Copilot January 6, 2026 18:30
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

Comment thread onnx/reference/ops/op_sigmoid.py Outdated
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

Comment thread onnx/reference/ops/op_sigmoid.py Outdated
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby requested a review from cbourjau January 6, 2026 18:38
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.25%. Comparing base (21f9f31) to head (7bec479).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7556   +/-   ##
=======================================
  Coverage   55.24%   55.25%           
=======================================
  Files         515      515           
  Lines       32354    32349    -5     
  Branches     2897     2894    -3     
=======================================
- Hits        17874    17873    -1     
+ Misses      13688    13686    -2     
+ Partials      792      790    -2     

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

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

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

@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Jan 6, 2026
@justinchuby justinchuby merged commit 69e3920 into main Jan 6, 2026
48 checks passed
@justinchuby justinchuby deleted the justinchu/vectorize branch January 6, 2026 23:13
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants