Gradient Operator by philipc2 · Pull Request #1313 · UXARRAY/uxarray · GitHub
Skip to content

Gradient Operator #1313

Merged
rajeeja merged 33 commits into
mainfrom
gradient
Sep 5, 2025
Merged

Gradient Operator #1313
rajeeja merged 33 commits into
mainfrom
gradient

Conversation

@philipc2

@philipc2 philipc2 commented Jul 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Closes #1299

Overview

  • Implements a UxDataArray.gradient() operator, which computes the zonal and meridional gradients
  • Develops a User Guide notebook showcasing the gradient operator.

@philipc2 philipc2 requested review from rajeeja and removed request for rajeeja July 7, 2025 19:11
@philipc2 philipc2 removed the request for review from rajeeja July 7, 2025 19:12
@philipc2 philipc2 added this to the SIParCS 2025 milestone Jul 7, 2025
@review-notebook-app

review-notebook-app Bot commented Jul 22, 2025

Copy link
Copy Markdown

@philipc2 philipc2 linked an issue Aug 12, 2025 that may be closed by this pull request
Quiver Plot Visualization Example #1301
Closed
@philipc2 philipc2 changed the title DRAFT: Gradient Operator Gradient Operator Aug 29, 2025
@philipc2 philipc2 marked this pull request as ready for review August 29, 2025 07:16
@philipc2

philipc2 commented Aug 29, 2025

Copy link
Copy Markdown
Member Author

@rajeeja @erogluorhan

This should be good for a review now.

@erogluorhan

erogluorhan commented Aug 29, 2025

Copy link
Copy Markdown
Member

@rajeeja @erogluorhan

This should be good for a review now.

Thanks, will do it today

@rajeeja

rajeeja commented Aug 29, 2025

Copy link
Copy Markdown
Contributor

@rajeeja @erogluorhan
This should be good for a review now.

Thanks, will do it today

+1

@philipc2

philipc2 commented Aug 29, 2025

Copy link
Copy Markdown
Member Author

Thank you both!

rajeeja
rajeeja approved these changes Aug 31, 2025
View reviewed changes

@rajeeja rajeeja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Thanks for the good work @egallmeier and @philipc2
minor comment inline for data inspection before operation

Comment thread uxarray/core/gradient.py
erogluorhan
erogluorhan reviewed Sep 2, 2025
View reviewed changes

@erogluorhan erogluorhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great to me; thanks a lot for all the great efforts @egallmeier and @philipc2 !

Only a couple things:

  • calculus.ipynb looks like a leftover that should probably be removed
  • Re new MPAS data and grid files for the gradients test (i.e. "subset" ones): are thopse subsets of existing files we have? Is the subsetting procedure complex that should not be in the testing script? I am trying to understand if we should really add them or generate on the fly.

@philipc2

philipc2 commented Sep 4, 2025

Copy link
Copy Markdown
Member Author

Thanks for the reviews @rajeeja and @erogluorhan

Re new MPAS data and grid files for the gradients test (i.e. "subset" ones): are thopse subsets of existing files we have? Is the subsetting procedure complex that should not be in the testing script? I am trying to understand if we should really add them or generate on the fly.

The grids are a subset of a 30km MPAS grid and are used for both testing and documentation. I chose to save subset since it's more reusable, especially in our testing.

Generating them on the fly would require us to load the entire original grid, which I'd prefer to avoid for our testing.

@erogluorhan

erogluorhan commented Sep 5, 2025

Copy link
Copy Markdown
Member

Depending on whether calculus.ipynb will be removed from this PR or not, I can approve it now or then

@philipc2

philipc2 commented Sep 5, 2025

Copy link
Copy Markdown
Member Author

erogluorhan
erogluorhan approved these changes Sep 5, 2025
View reviewed changes

@erogluorhan erogluorhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great!

@rajeeja rajeeja merged commit 72b88f3 into main Sep 5, 2025
15 checks passed
@erogluorhan erogluorhan deleted the gradient branch September 26, 2025 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rajeeja rajeeja rajeeja approved these changes
@erogluorhan erogluorhan erogluorhan approved these changes

Assignees

@philipc2 philipc2

@egallmeier egallmeier

Labels

None yet

Projects

None yet

Milestone

Calculus Operators

Development

Successfully merging this pull request may close these issues.

Quiver Plot Visualization Example Add Gradients

5 participants

@philipc2 @erogluorhan @rajeeja @rljacob @egallmeier