Skip to content

Conversation

WanHsuanLin
Copy link

  1. Add implementation for subgrid_shift_x and subgrid_shift_y as described in Shift subset of positions defined by a list of indices. #31
  2. Add assertion to check if the shift operation causes column/row order changes
  3. Can take ilist.IList and slice as input for column/row indices
  4. Add test cases.

@weinbe58 weinbe58 requested a review from Copilot October 14, 2025 22:06
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link

@Copilot 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 implements subgrid shift functionality for Grid objects, allowing selective shifting of columns and rows while preserving grid integrity. The implementation adds methods to shift specific column or row indices by a given amount while automatically adjusting spacing to maintain grid structure.

  • Adds shift_subgrid_x and shift_subgrid_y methods with support for both ilist.IList and slice inputs
  • Implements validation to prevent invalid shifts that would change column/row ordering
  • Includes comprehensive test coverage for various scenarios including edge cases

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/bloqade/geometry/dialects/grid/types.py Core implementation of subgrid shift methods with spacing adjustment logic
src/bloqade/geometry/dialects/grid/stmts.py Statement definitions for the new shift operations
src/bloqade/geometry/dialects/grid/concrete.py Concrete interpreter implementations for subgrid shift statements
src/bloqade/geometry/dialects/grid/_interface.py Public API interface definitions with documentation
src/bloqade/geometry/dialects/grid/init.py Module exports for new shift operations
test/grid/test_types.py Comprehensive test cases covering various input types and edge cases
test/grid/test_concrete.py Integration tests for concrete interpreter functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Args:
grid (Grid): a grid object
y_indices (float): a list/ilist of x indices to shift
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Parameter description is incorrect. The parameter y_indices should be described as 'a list/ilist of y indices to shift', not 'x indices'.

Suggested change
y_indices (float): a list/ilist of x indices to shift
y_indices (float): a list/ilist of y indices to shift

Copilot uses AI. Check for mistakes.

Args:
grid (Grid): a grid object
x_indices (float): a list/ilist of x indices to shift
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Parameter type annotation is incorrect. The parameter x_indices should be described as having type ilist.IList[int, typing.Any], not float.

Suggested change
x_indices (float): a list/ilist of x indices to shift
x_indices (ilist.IList[int, typing.Any]): a list/ilist of x indices to shift

Copilot uses AI. Check for mistakes.

Args:
grid (Grid): a grid object
y_indices (float): a list/ilist of y indices to shift
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Parameter type annotation is incorrect. The parameter y_indices should be described as having type ilist.IList[int, typing.Any], not float.

Suggested change
y_indices (float): a list/ilist of y indices to shift
y_indices (ilist.IList[int, typing.Any]): a list/ilist of y indices to shift

Copilot uses AI. Check for mistakes.

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.

1 participant