Adapt py2fgen for gt4py programs #371

samkellerhals · 2024-01-31T11:22:55Z

Description

I've refactored py2fgen to enhance its functionality and integrate it more seamlessly with Fortran.

Key changes include:

Integration Tests: Added tests with Fortran driver codes for straightforward scenarios to ensure that code generated with py2fgen compiles, runs as expected from Fortran, and delivers the anticipated results.
Python Wrapper Function Generation: Modified py2fgen to automatically generate a Python wrapper function for the exposed Python function, applicable to both gt4py.Program and Simple Python function types.
New Options for Debug Mode and gt4py Backend Selection: Introduced options to enable debug mode and select the gt4py backend. See the README for more details.
Singular Function Binding Generation: Altered the tool to generate bindings and libraries for one function at a time, streamlining the process.
Removal of @CffiMethod and @to_fields Decorators: These decorators have been removed to simplify the usage and enhance the tool's compatibility.

Future Work

Expand the test suite, the code coverage is quite low, and we need to test more edge cases.
Determine what the overhead is of invoking a Python interpreter from CFFI-Fortran.
Test if this works with more complex cases, for example the Diffusion granule.
Check if we can determine array size in the f90 interface using SIZE similar to how it is being handled in the bindings generator here:

icon4py/tools/src/icon4pytools/icon4pygen/bindings/codegen/f90.py

Line 409 in f8ca7be

right_side=f"SIZE({field.name}, 2)",

samkellerhals · 2024-02-06T16:08:10Z

cscs-ci run default

…testing

samkellerhals · 2024-02-08T17:42:01Z

cscs-ci run default

samkellerhals · 2024-02-14T14:33:46Z

cscs-ci run default

samkellerhals · 2024-02-15T09:52:03Z

cscs-ci run default

samkellerhals · 2024-02-15T10:09:33Z

cscs-ci run default

samkellerhals · 2024-02-15T13:30:02Z

cscs-ci run default

samkellerhals · 2024-02-15T15:04:02Z

cscs-ci run default

samkellerhals · 2024-02-16T11:56:38Z

cscs-ci run default

samkellerhals · 2024-02-19T08:36:25Z

cscs-ci run default

samkellerhals · 2024-02-19T15:59:59Z

cscs-ci run default

samkellerhals · 2024-02-19T16:19:57Z

cscs-ci run default

samkellerhals · 2024-02-19T16:43:38Z

launch jenkins spack

github-actions · 2024-02-26T13:18:23Z

Mandatory Tests

Please make sure you run these tests via comment before you merge!

cscs-ci run default
launch jenkins spack

Optional Tests

To run benchmarks you can use:

cscs-ci run benchmark

To run tests and benchmarks with the DaCe backend you can use:

cscs-ci run dace

In case your change might affect downstream icon-exclaim, please consider running

launch jenkins icon

For more detailed information please look at CI in the EXCLAIM universe.

samkellerhals · 2024-02-26T13:32:55Z

cscs-ci run default

halungge

Lets discuss it, I am not sure I did understand everything.

halungge · 2024-02-27T08:21:51Z

tools/src/icon4pytools/py2fgen/parsing.py

+def _parse_function(module: ModuleType, function_name: str) -> Func:
+    func = unwrap(getattr(module, function_name))
+    is_gt4py_program = isinstance(func, Program)
+    type_hints = _extract_type_hint_strings(module, func, is_gt4py_program, function_name)


do you relay on the type hints being GT4Py type hints, even for non-program case? If yes, you should add that as a restriction to the README.

I did mention it here https://github.com/C2SM/icon4py/pull/371/files#diff-9cea6a1607559a2e7aefc9e317dbd94ee9c9c8488e24f0e86074509cd2850090R350

halungge · 2024-02-27T08:24:48Z

tools/src/icon4pytools/py2fgen/template.py

+
+
+class CffiPlugin(Node):
+    module_name: str


Can you make these (frozen) dataclasses? Or is this already the case from the Node inheritance?

I'm not sure it makes sense for these to be frozen dataclasses as the attributes change after initialisation post_init etc. For the purposes of code generation with eve I think just creating subclasses of eve.Nodes is generally the right approach.

halungge · 2024-02-27T08:31:47Z

tools/src/icon4pytools/py2fgen/template.py

+    # Allocate GT4Py Fields
+    {% for arg in _this_node.function.args %}
+    {% if arg.is_array %}
+    {{ arg.name }} = np_as_located_field({{ ", ".join(arg.gtdims) }})({{ arg.name }})


isn't this one (np_as_located_field ) deprecated and we should use as_field instead?

I agree in principle however when I tried using as_field for some reason the data that was returned when I tried to print the arrays on the Python side was just filled with garbage values. I'm not sure if I used as_field wrongly before or if some implementation detail changed between those two functions on the gt4py side but it is definitely something I would like to investigate in the new cycle project (will add it to the hackmd doc).

They changed the representation of the buffers. Before it Field[CellDim, KDim] was in C like style, the k had stride 1, now it is the otherway round. That needed a fix in GHEX.

halungge · 2024-02-27T08:35:59Z

tools/src/icon4pytools/py2fgen/template.py

+from gt4py.next.iterator.embedded import np_as_located_field
+from gt4py.next.program_processors.runners.gtfn import run_gtfn, run_gtfn_gpu
+from gt4py.next.program_processors.runners.roundtrip import backend as run_roundtrip
+from icon4py.model.common.grid.simple import SimpleGrid


Could you use the BaseGrid here? You cannot instantiate it later, I guess. But when running the diffusion you will need the IconGrid. The instantiation needs to be injected somehow.

You could maybe use the same pattern as for the backends.

I'm not sure I understand, isn't the grid instantiated inside the function that runs the diffusion granule code? e.g.

grid = IconGrid().with_config(mesh_config).with_connectivities(connectivities)

With this PR I think the focus should just be on these two simple use cases (square, multi_return), once we want to support running the granules as well we can make the necessary changes to the code generation. However I first need to understand how to call the diffusion code from Python correctly myself. I've updated the hackmd here: https://hackmd.io/Ri-3S3joQ5SebR5A3_rH5g#Granule-CFFI-Plugin-Generation

You import the SimpleGrid and then later on in the template you have a grid = SimpleGrid() So that only works when you run your function on the simple grid. If you only used the common base class (that is only rely on the interface of the grid, it would work with whatever implementation you need. But maybe the the template is not meant that general.

tools/src/icon4pytools/py2fgen/wrappers/simple.py

tools/src/icon4pytools/py2fgen/utils.py

samkellerhals · 2024-02-27T13:41:22Z

launch jenkins spack

halungge

I approve, as we discussed for me its fine to merge, since you continue to work on it any. You should have a look at the as_field again, maybe the changed representation hit you there.

halungge · 2024-02-27T16:25:06Z

tools/src/icon4pytools/py2fgen/template.py

+    # Allocate GT4Py Fields
+    {% for arg in _this_node.function.args %}
+    {% if arg.is_array %}
+    {{ arg.name }} = np_as_located_field({{ ", ".join(arg.gtdims) }})({{ arg.name }})


They changed the representation of the buffers. Before it Field[CellDim, KDim] was in C like style, the k had stride 1, now it is the otherway round. That needed a fix in GHEX.

halungge · 2024-02-27T16:30:38Z

tools/src/icon4pytools/py2fgen/template.py

+from gt4py.next.iterator.embedded import np_as_located_field
+from gt4py.next.program_processors.runners.gtfn import run_gtfn, run_gtfn_gpu
+from gt4py.next.program_processors.runners.roundtrip import backend as run_roundtrip
+from icon4py.model.common.grid.simple import SimpleGrid


You import the SimpleGrid and then later on in the template you have a grid = SimpleGrid() So that only works when you run your function on the simple grid. If you only used the common base class (that is only rely on the interface of the grid, it would work with whatever implementation you need. But maybe the the template is not meant that general.

Adapt py2fgen for single functions and gt4py programs

a494b98

samkellerhals marked this pull request as draft January 31, 2024 11:23

samkellerhals changed the title ~~Adapt py2fgen for single functions and gt4py programs~~ Adapt py2fgen for gt4py programs Jan 31, 2024

samkellerhals added 6 commits February 1, 2024 10:42

hacky fixes for codegen, compilable dl

d2016d2

Interrim working

593eff8

Change wrapper name

8c00135

Change wrapper name

3f3a3d3

Working compile and run, size arguments specified in wrapper

7c6c1e6

cleanup

8c800ce

samkellerhals added 2 commits February 8, 2024 18:28

Add integration test

6c40d1e

Merge branch 'main' of github.com:C2SM/icon4py into py2f-integration-…

0bdb4da

…testing

samkellerhals added 5 commits February 9, 2024 19:35

Generate wrapper (WIP) 🚧

1f1f922

Generate wrapper

f4045ad

Generate wrappers directly for gt4py programs

416ae16

Add more test cases

d782fdf

Add more test cases

46731f4

Change gt4py branch temporarily to check for speedup

3045bee

add gfortran to Docker image

b865bb9

Cleanup code

3eeaef2

Extract all dimension types to support codegen for all icon4py stencils

05f176d

Cleanup tests and Fortran drivers

708a55f

More improvements and cleanup

a2ea092

Update README.md

5e2c50f

Fix CLI and update README.md

21e4cfa

samkellerhals marked this pull request as ready for review February 19, 2024 16:43

samkellerhals requested review from halungge and abishekg7 February 19, 2024 16:43

samkellerhals added 2 commits February 26, 2024 14:03

Pull from main

c7d5eaa

Fix LiftMode enum attr access

d40e43b

halungge requested changes Feb 27, 2024

View reviewed changes

samkellerhals requested a review from halungge February 27, 2024 11:41

halungge approved these changes Feb 27, 2024

View reviewed changes

samkellerhals merged commit 5d94f3e into main Feb 28, 2024
5 checks passed

samkellerhals deleted the py2f-integration-testing branch February 28, 2024 07:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt py2fgen for gt4py programs #371

Adapt py2fgen for gt4py programs #371

samkellerhals commented Jan 31, 2024 •

edited

Loading

samkellerhals commented Feb 6, 2024

samkellerhals commented Feb 8, 2024

samkellerhals commented Feb 14, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 16, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

github-actions bot commented Feb 26, 2024

samkellerhals commented Feb 26, 2024

halungge left a comment

halungge Feb 27, 2024

samkellerhals Feb 27, 2024

halungge Feb 27, 2024

samkellerhals Feb 27, 2024

halungge Feb 27, 2024

samkellerhals Feb 27, 2024

halungge Feb 27, 2024

halungge Feb 27, 2024

halungge Feb 27, 2024

samkellerhals Feb 27, 2024 •

edited

Loading

halungge Feb 27, 2024

samkellerhals commented Feb 27, 2024

halungge left a comment

halungge Feb 27, 2024

halungge Feb 27, 2024

Adapt py2fgen for gt4py programs #371

Adapt py2fgen for gt4py programs #371

Conversation

samkellerhals commented Jan 31, 2024 • edited Loading

Description

Key changes include:

Future Work

samkellerhals commented Feb 6, 2024

samkellerhals commented Feb 8, 2024

samkellerhals commented Feb 14, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 15, 2024

samkellerhals commented Feb 16, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

samkellerhals commented Feb 19, 2024

github-actions bot commented Feb 26, 2024

samkellerhals commented Feb 26, 2024

halungge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samkellerhals Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samkellerhals commented Feb 27, 2024

halungge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samkellerhals commented Jan 31, 2024 •

edited

Loading

samkellerhals Feb 27, 2024 •

edited

Loading