Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for NCHW layout in stablehlo.composites for jax.image.resize and torch.nn.interpolate in nearest mode. #68205

Closed
wants to merge 0 commits into from

Conversation

copybara-service[bot]
Copy link

Add support for NCHW layout in stablehlo.composites for jax.image.resize and torch.nn.interpolate in nearest mode.

Copy link

@NIV27e NIV27e left a comment

Choose a reason for hiding this comment

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

Great job on this PR! Adding support for the NCHW layout in stablehlo.composites for jax.image.resize and torch.nn.interpolate in nearest mode is a significant improvement.

Composite-Lowering.mlir
The is_nchw_op attribute addition is well done. This clearly differentiates between NCHW and NHWC operations. Just ensure all edge cases are covered.

Composite_Lowering_Patterns.td
The new constraints and patterns for NCHW are clearly defined and seem to work well. Make sure they’re thoroughly tested with various inputs.

Composite_Utils.cc and Composite_Utils.h
The removal of IsSupportedNchwUpsampleBlinear and related utilities makes sense if they’re no longer needed. Just double-check that all utility functions handle NCHW layout without performance issues.

Your comments are clear and helpful. If possible, simplify any complex logic to enhance readability. Consider profiling the changes to avoid any performance hits, especially in critical paths.

Questions

  1. Are there specific test cases for verifying the new NCHW layout handling?
  2. How do these changes affect workflows that rely on NHWC layout? Any backward compatibility issues?

Overall, this is solid work! Looking forward to seeing it merged.

@copybara-service copybara-service bot closed this May 29, 2024
@copybara-service copybara-service bot deleted the exported_pr_634987803 branch May 29, 2024 18:33
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.

None yet

2 participants