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.

Sign up for GitHub

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

Jump to bottom

Add transform metadata to describe arrays' axes types #284

Merged
merged 3 commits into from
Jun 11, 2022
Merged

Add transform metadata to describe arrays' axes types #284

merged 3 commits into from
Jun 11, 2022

Conversation

kaushikcfd
Copy link
Collaborator

@kaushikcfd kaushikcfd commented Nov 8, 2021 •
edited by inducer
Loading

Draft because:

  • Fails CI.
  • ⚠️ DROP arraycontext req.txt commit before merging

@kaushikcfd kaushikcfd changed the title Add transform metadata to describe arrays' types Add transform metadata to describe arrays' axes types Nov 8, 2021
inducer
inducer reviewed Nov 26, 2021
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! A few questions below.

meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Nov 26, 2021

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

@kaushikcfd kaushikcfd force-pushed the axis_metadata branch 2 times, most recently from 0b9c42c to 26c9f8d Compare March 3, 2022 22:53
@kaushikcfd kaushikcfd requested a review from inducer March 3, 2022 23:07
inducer
inducer reviewed Mar 7, 2022
View reviewed changes
meshmode/discretization/__init__.py Outdated Show resolved Hide resolved
meshmode/discretization/__init__.py Outdated Show resolved Hide resolved
This was referenced Mar 12, 2022
Closed
Closed
inducer
inducer reviewed Mar 28, 2022
View reviewed changes
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
@kaushikcfd kaushikcfd force-pushed the axis_metadata branch 2 times, most recently from dde4973 to 58c429e Compare April 4, 2022 22:03
@kaushikcfd
Copy link
Collaborator Author

@inducer: Thanks for the suggestions! I've made this patch leaner to make it more accurate.

@kaushikcfd kaushikcfd marked this pull request as draft April 4, 2022 22:36
@kaushikcfd kaushikcfd marked this pull request as ready for review April 5, 2022 06:11
@kaushikcfd kaushikcfd requested a review from inducer April 5, 2022 06:11
inducer
inducer reviewed Apr 17, 2022
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! A few comments and suggestions below.

meshmode/array_context.py Show resolved Hide resolved
meshmode/array_context.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Apr 17, 2022

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

@inducer inducer mentioned this pull request Jun 10, 2022
Merged
@inducer
Copy link
Owner

inducer commented Jun 10, 2022

FYI I'm working on getting this merge-ready, so no need to mess with it ATM. :)

inducer
inducer approved these changes Jun 10, 2022
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I've made some changes to get this ship shape. @kaushikcfd Please take a look to see if you like my changes, and if so, this is ready to go. I can take care of updating inducer/grudge#188 afterwards.

meshmode/transform_metadata.py Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/transform_metadata.py Outdated Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
meshmode/discretization/connection/modal.py Outdated Show resolved Hide resolved
meshmode/array_context.py Show resolved Hide resolved
@kaushikcfd
Copy link
Collaborator Author

kaushikcfd commented Jun 11, 2022 •
edited
Loading

Force-pushed to resolve conflicts with #334.

kaushikcfd
kaushikcfd commented Jun 11, 2022
View reviewed changes
Copy link
Collaborator Author

@kaushikcfd kaushikcfd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@inducer
Copy link
Owner

inducer commented Jun 11, 2022

Thanks for taking a look! Could you take a (hopefully quick) look at inducer/arraycontext#171, since this needs that?

@inducer
Copy link
Owner

inducer commented Jun 11, 2022

Argh, nvm! Was missing a reload. Sorry!

@inducer inducer enabled auto-merge (rebase) June 11, 2022 22:16
@inducer inducer merged commit dff7d4d into inducer:main Jun 11, 2022
@inducer inducer deleted the axis_metadata branch June 12, 2022 00:35
@inducer inducer mentioned this pull request Jun 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inducer inducer inducer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kaushikcfd @inducer