Skip to content

Extend level and field_values to work with lazy Fields - #2356

Merged
dennisYatunin merged 1 commit into
mainfrom
dy/lazy_field_levels
Jul 19, 2025
Merged

Extend level and field_values to work with lazy Fields#2356
dennisYatunin merged 1 commit into
mainfrom
dy/lazy_field_levels

Conversation

@dennisYatunin

@dennisYatunin dennisYatunin commented Jul 17, 2025

Copy link
Copy Markdown
Member

This PR extends level and field_values to work with Broadcasted expressions that represent lazy pointwise field operations. This builds on the work from #2331, which extended column and slab to work with lazy fields. In order to test field_values with lazy fields, it was also necessary to define == for DataLayouts.

  • Code follows the style guidelines OR N/A.
  • Unit tests are included OR N/A.
  • Code is exercised in an integration test OR N/A.
  • Documentation has been added/updated OR N/A.

@imreddyTeja imreddyTeja 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.

At some point we may want to add documentation for the different broadcast style types, even though most of the names are self explanatory. I also couldn't find much about NonExtrudedBroadcasted, and I'm not exactly sure what it does, or where it is used.

Comment thread test/Fields/unit_field.jl
@akshaysridhar

Copy link
Copy Markdown
Member

At some point we may want to add documentation for the different broadcast style types, even though most of the names are self explanatory. I also couldn't find much about NonExtrudedBroadcasted, and I'm not exactly sure what it does, or where it is used.

The type-name suggests that NonExtrudedBroadcasted is used only with the ClimaCore.DataLayouts.IJFH layouts (in ClimaCore there is a 2D example problem which uses RectangleDomains in the bickley_jet directory.

@dennisYatunin

Copy link
Copy Markdown
Member Author

My plan for this section of code is to eventually remove it altogether, since it is just duplicating the array slicing code in DataLayouts.jl for broadcast expressions. If we appropriately abstract out the slicing logic, we should not need to duplicate all of it like this (and we should not have this issue of some slicing being implemented for arrays but not for broadcasts). More generally, I think the slicing code can be simplified to a single method that unrolls over DataLayout axes, so that we don't need to have three different methods (column/level/slab) for each of our 15 available layouts. Reducing all of the slicing code in this module to one method should make documentation and unit testing much easier.

And I'm also not sure what NonExtrudedBroadcasted is or what it does, since it doesn't seem to be documented anywhere. At some point I will look into whether it is necessary/useful.

@dennisYatunin

dennisYatunin commented Jul 18, 2025

Copy link
Copy Markdown
Member Author

@akshaysridhar I think the word "extruded" in this context refers to broadcast axis extrusion (e.g., the ability to do something like vector .+ matrix, where the second axis of vector is extruded to match the second axis of matrix), rather than the extrusion of our horizontal ClimaCore spaces into 3D spaces.

@akshaysridhar

Copy link
Copy Markdown
Member

@akshaysridhar I think the word "extruded" in this context refers to broadcast axis extrusion (e.g., the ability to do something like vector .+ matrix, where the second axis of vector is extruded to match the second axis of matrix), rather than the extrusion of our horizontal ClimaCore spaces into 3D spaces.

Ah; noted. thanks.

@dennisYatunin
dennisYatunin force-pushed the dy/lazy_field_levels branch from 0138781 to cc7bdd1 Compare July 18, 2025 22:04
@dennisYatunin
dennisYatunin force-pushed the dy/lazy_field_levels branch from cc7bdd1 to 11875a0 Compare July 18, 2025 22:05
@dennisYatunin dennisYatunin changed the title Extend level to work with lazy Fields Extend level and field_values to work with lazy Fields Jul 18, 2025
@dennisYatunin
dennisYatunin enabled auto-merge July 18, 2025 23:36
@dennisYatunin
dennisYatunin merged commit 79e9e05 into main Jul 19, 2025
35 checks passed
@dennisYatunin
dennisYatunin deleted the dy/lazy_field_levels branch July 19, 2025 04:41
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.

4 participants