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

Changes to test configurations options and externals for parsing #53

Merged
merged 11 commits into from
Jun 20, 2024
+6 −7
Merged

Changes to test configurations options and externals for parsing #53

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
164120b
Changed conditional statement to check for compute option in test_tra…
fmalatino Jun 7, 2024
a8e58e6
Linting
fmalatino Jun 7, 2024
170e47a
Updating submodules
fmalatino Jun 7, 2024
94b1079
Updating dace
fmalatino Jun 11, 2024
6999653
Changed get_communicator conditional to check for cubed-sphere
fmalatino Jun 11, 2024
e2bd82f
Changed get_communicator topology selection conditional
fmalatino Jun 12, 2024
50c8bc7
Changed mysign in fill_corners_dgrid_def to external variable
fmalatino Jun 12, 2024
7db441a
Linting
fmalatino Jun 12, 2024
ad6c500
Rolling back dace and gt4py
fmalatino Jun 13, 2024
2a5c9f4
Removing mysign from axis_offsets
fmalatino Jun 13, 2024
fc21a7d
Merge branch 'develop' into fix/computegridoption
fmalatino Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion external/dace
Open in desktop
Submodule dace updated 8 files
+4 −2 .github/workflows/pyFV3-ci.yml
+1 −1 dace/config_schema.yml
+2 −0 dace/sdfg/sdfg.py
+7 −0 dace/sdfg/state.py
+36 −1 dace/transformation/passes/analysis.py
+88 −39 dace/viewer/templates/sdfv.html
+1 −1 dace/viewer/webclient
+49 −0 tests/passes/sdfg_constraint_derivation_test.py
2 changes: 1 addition & 1 deletion external/gt4py
Open in desktop
Submodule gt4py updated 1 files
+1 −2 src/gt4py/next/program_processors/runners/dace_iterator/itir_to_sdfg.py
1 change: 1 addition & 0 deletions ndsl/dsl/stencil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ def axis_offsets(
"local_js": gtscript.J[0] + self.jsc - origin[1],
"j_end": j_end,
"local_je": gtscript.J[-1] + self.jec - origin[1] - domain[1] + 1,
"mysign": -1.0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is too aggressive as a fix. The axis_offsets function shouldn't carry the mysign issue.

}

def get_origin_domain(
Expand Down
3 changes: 1 addition & 2 deletions ndsl/stencils/corners.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,6 @@ def fill_corners_dgrid_defn(
x_out: FloatField,
y_in: FloatField,
y_out: FloatField,
mysign: float,
):
"""
Args:
Expand All @@ -998,7 +997,7 @@ def fill_corners_dgrid_defn(
y_in (in):
y_out (inout):
"""
from __externals__ import i_end, i_start, j_end, j_start
from __externals__ import i_end, i_start, j_end, j_start, mysign
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pass it as a default externals or directly a scalar, since never changes in our code


with computation(PARALLEL), interval(...):
# sw corner
Expand Down
8 changes: 4 additions & 4 deletions ndsl/stencils/testing/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ def pytest_addoption(parser):
parser.addoption(
"--topology",
action="store",
default="cube-sphere",
help='Topology of the grid. "cube-sphere" means a 6-faced grid, "doubly-periodic" means a 1 tile grid. Default to "cube-sphere".',
default="cubed-sphere",
help='Topology of the grid. "cubed-sphere" means a 6-faced grid, "doubly-periodic" means a 1 tile grid. Default to "cubed-sphere".',
)


Expand Down Expand Up @@ -189,7 +189,7 @@ def get_ranks(metafunc, layout):
if only_rank is None:
if topology == "doubly-periodic":
total_ranks = layout[0] * layout[1]
elif topology == "cube-sphere":
elif topology == "cubed-sphere":
total_ranks = 6 * layout[0] * layout[1]
else:
raise NotImplementedError(f"Topology {topology} is unknown.")
Expand Down Expand Up @@ -370,7 +370,7 @@ def generate_parallel_stencil_tests(metafunc, *, backend: str):


def get_communicator(comm, layout, topology_mode):
if (MPI.COMM_WORLD.Get_size() > 1) and (topology_mode == "doubly-periodic"):
if (MPI.COMM_WORLD.Get_size() > 1) and (topology_mode == "cubed-sphere"):
Copy link

Choose a reason for hiding this comment

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

This change will definitely make a big difference!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was a bug I introduce in the PR right before

partitioner = CubedSpherePartitioner(TilePartitioner(layout))
communicator = CubedSphereCommunicator(comm, partitioner)
else:
Expand Down
2 changes: 1 addition & 1 deletion ndsl/stencils/testing/test_translate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ def test_parallel_savepoint(
)
if case.testobj.skip_test:
return
if grid and not case.testobj.compute_grid_option:
if (grid == "compute") and not case.testobj.compute_grid_option:
pytest.xfail(f"Grid compute option not used for test {case.savepoint_name}")
input_data = dataset_to_dict(case.ds_in)
# run python version of functionality
Expand Down
Loading