Skip to content

Conversation

@weinbe58
Copy link
Member

@weinbe58 weinbe58 commented May 9, 2025

This PR implements the type inference from the grid.New statement since the grid shape is equal to (Nx+1, Ny+1) where (Nx, Ny) comes from the input arguments.

@weinbe58 weinbe58 requested a review from Copilot May 9, 2025 18:57
@weinbe58 weinbe58 merged commit 751e542 into main May 9, 2025
9 checks passed
Copy link
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 adds type inference for the grid.New statement so that a grid’s dimensions become (Nx+1, Ny+1) based on the input list lengths.

  • Updates the test to verify that grid.New returns a GridType with dimensions inferred from the input list lengths.
  • Adjusts the grid.New statement signature to use type variables and introduces a new type inference method in _typeinfer.py.
  • Registers the new type inference feature in the grid dialect initialization.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/grid/test_typeinfer.py Adds a test to validate type inference of grid.New
src/bloqade/geometry/prelude.py Introduces a typeinfer flag in the geometry DSL pass
src/bloqade/geometry/dialects/grid/stmts.py Updates type annotations for spacing and result with type variables
src/bloqade/geometry/dialects/grid/_typeinfer.py Implements type inference logic by computing grid dimensions based on input lengths
src/bloqade/geometry/dialects/grid/init.py Registers TypeInferMethods in the grid dialect
Comments suppressed due to low confidence (1)

src/bloqade/geometry/dialects/grid/stmts.py:29

  • [nitpick] The type variable used for x_spacing ('NumXStep') and y_spacing ('NumYStep') is different from the result type variables ('NumX' and 'NumY'). Consider aligning these names to improve clarity and reduce potential confusion in type inference.
x_spacing: ir.SSAValue = info.argument(
        type=ilist.IListType[types.Float, types.TypeVar("NumXStep")]
    )

class TypeInferMethods(MethodTable):

def get_len(self, typ: types.TypeAttribute):
if typ.is_subseteq(ilist.IListType[types.Int, types.Any]):
Copy link

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

The get_len function checks for a list of integers while grid.New declares spacing as a list of floats. Verify that this discrepancy is intentional or update the types to ensure consistency in type inference.

Suggested change
if typ.is_subseteq(ilist.IListType[types.Int, types.Any]):
if typ.is_subseteq(ilist.IListType[types.Int, types.Any]) or typ.is_subseteq(ilist.IListType[types.Float, types.Any]):

Copilot uses AI. Check for mistakes.
@weinbe58 weinbe58 deleted the phil/update-typeinfer branch May 9, 2025 18:57
weinbe58 added a commit that referenced this pull request May 9, 2025
* adding type infer for

* making type check a bit tighter bound
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