Skip to content

Conversation

kistenklaus
Copy link
Contributor

See issue #2539 for a better explanation.

I know crazy stuff right =^).

@justinchuby justinchuby added the module: torchlib Related to the torch/aten function lib in development label Sep 4, 2025
Copy link
Collaborator

@justinchuby justinchuby left a comment

Choose a reason for hiding this comment

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

Good catch, thank you!

@github-project-automation github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Sep 4, 2025
@justinchuby justinchuby changed the title Fixes dynamic padding bug. [torch] Fix incorrect inputs to Concat when processing dynamic paddings Sep 4, 2025
@justinchuby justinchuby changed the title [torch] Fix incorrect inputs to Concat when processing dynamic paddings [torch] Fix incorrect Concat when processing dynamic paddings Sep 4, 2025
@justinchuby justinchuby enabled auto-merge (squash) September 4, 2025 21:45
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.99%. Comparing base (fc792e4) to head (dba112f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
onnxscript/function_libs/torch_lib/ops/nn.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2540   +/-   ##
=======================================
  Coverage   69.99%   69.99%           
=======================================
  Files         216      216           
  Lines       26074    26074           
  Branches     2618     2618           
=======================================
  Hits        18250    18250           
  Misses       6921     6921           
  Partials      903      903           

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

@justinchuby justinchuby merged commit d98e3dd into microsoft:main Sep 4, 2025
32 checks passed
@titaiwangms
Copy link
Contributor

Good catch, thank you!

This is a serious bug. Dis we never use this op in LLM? We missed it until now.

@justinchuby
Copy link
Collaborator

The models we have seen probably did not pad dynamically

justinchuby added a commit that referenced this pull request Sep 4, 2025
This is a follow up of #2540
to add a test described in
#2539.

Fix #2539

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: torchlib Related to the torch/aten function lib in development
Projects
Development

Successfully merging this pull request may close these issues.

3 participants