Read surface topography from Elmer-Xios output by ClemensSchannwell · Pull Request #12 · EBFMorg/EBFM

ClemensSchannwell · 2025-10-23T13:22:06Z

In order to read the surface topography from an actual Elmer/Ice simulation, I included the option to --netcdf-mesh-unstructured. Since this surface topography uses the library XIOS to write the file, it lives on the same grid as Elmer. This simplifies the reading of the file as the surface topography comes in the same order as the Elmer mesh.
I think this feature will be helpful in the case of initialisation, because we can read the same surface topography in with which Elmer is initialised and also if we decide that we do not need to update the surface topography within a EBFM run, we can simply read it it at the start of the next simulation.
I also wanted to write some model output for easy inspection using ncview or similar, but while it works in principle, this is still in its early stage. This does not hold up further development of the EBFM-dummy but would be good to have to check whether results from EBFM are physically plausible.
I made some test with the feature_greenland branch and all previous documented running commands of the model still work as before (as far as I can tell at least).

To test the new model on levante, you can run:

mpirun -np 1 ebfm --elmer-mesh /work/bm1514/data/ebfm_dummy/mesh/MESH --netcdf-mesh-unstructured /work/bm1514/data/elmer-xios-output/restart.nc

The elmer-xios file should be accessible for all project members on levante.

BenjaminRodenberg

I merged main into this branch and cleaned up some formatting errors (will push in a moment). One part is still problematic (see comment below).

We introduced some checks in #13 and #14. These should help to have a uniform formatting and make sure all code is properly licensed. Please refer to https://github.com/wardvp/EBFM?tab=readme-ov-file#developer-notes for documentation.

BenjaminRodenberg · 2025-11-14T11:21:15Z

I did another merge with main to avoid that the two branches diverge too much.

Main change: src/dummy.py has been renamed to src/main.py. It's also generally recommended to install the package with pip install -e . now if you are developing and use the ebfm application for running (this will hide implementation details like src/main.py).

The last commit before the merge was 29350ad (just in case the merge causes trouble and you need to go back).

BenjaminRodenberg

I reviewed the design of this PR and provided some general comments.

I can also take care of the implementation of (some of) these parts if you want. Please just let me know in the comments.

BenjaminRodenberg · 2025-11-25T10:44:27Z



-def main(OUTFILE, io, OUT, grid, t, time, C):
+def main(OUTFILE, io, OUT, grid, t, time, C, gridtype):


Is it possible to use grid["input_type"] here? Introducing a new type is no problem in my opinion. Extending the interface is something I would only do if necessary.

So this and everything that is related to changes in the save_netCDF_file(gridtype) function is something, we should probably discuss whether it belongs to this pull request. I started implementing writing, a netcdf output file, but did not finish the work. So the question now is whether, I scratch the code bits that belong to this and open a new PR once this is merged or whether I keep this "unfinished" work in there. I am thinking the former is the better option.

I assume this is a problem originating from the general design of the code. The design is not very extensible yet.

Can you provide a link to your sketch? This helps me to better understand what you are trying to achieve with the writer and how we could update the design to make it a bot more extensible and avoid merge conflicts etc.

(maybe you noticed: I tried to follow a similar goal with #44 to decouple the configuration from the implementation in order to avoid merge conflicts and allow for easier parallel development)

BenjaminRodenberg · 2025-11-25T10:50:51Z

        return True

-    def save_netCDF_file():
+    def save_netCDF_file(gridtype):


This function is a bit problematic in my opinion due to the variables that are used from the outer scope (this makes the input/output hard to understand). Doing a big refactoring of the codebase is out of scope. But I think we could start here (since we are touching this function) by turning this function from a nested function (inside main) into an independent function on the root level of this file. I.e.:

Not

def main(...): ... elif io["output_type"] == 2: save_netCDF_file(gridtype=gridtype) ... def save_netCDF_file(gridtype): ...

but

def save_netCDF_file(gridtype, io, ...): ... def main(...): ... elif io["output_type"] == 2: save_netCDF_file(gridtype=gridtype, io=io, ...) ...

After some refactoring this problem still exists but it's actually not really the scope of this PR (it's already a problem of the original code). I would ignore this problem for now and do the refactoring at some later point when it fits better.

@ClemensSchannwell feel free to resolve this discussion if you agree.

BenjaminRodenberg · 2025-11-25T11:00:01Z

-            IN["h"] = data_from_elmer["h"]
-            grid["z"] = IN["h"][0].ravel()
-            # TODO add gradient field later
+            # IN['h'] = data_from_elmer['h']
+            # grid['z'] = IN['h'][0].ravel()


I assume we need to revert this in order to not break existing functionality?

Why did you have to comment it out in the first place? Is this something that is easy to fix or should we add a guard around this part? I.e.

if grid["grid_type"] == GridInputType.ELMERXIOS: logger.warning(f"Grid type {grid["grid_type"]} does not support outputting IN["h"] and grid["z"]. Ignoring these fields.") # todo: find a fix if necessary else: # usual behavior

Yes, we can revert this. It does actually not break anything. Since I am passing 42 from Elmer, I simply wanted to see whether it would do something more sensible if I did not update the topography (h in this case). Generally, a little further down the line it might actually not bad to also allow a flag --with-topography-update or similar.

BenjaminRodenberg · 2025-11-25T11:03:19Z

        grid["slope_gamma"][(grid["slope_x"] > 0) & (grid["slope_y"] == 0)] = np.pi / 2
        grid["slope_gamma"][(grid["slope_x"] < 0) & (grid["slope_y"] == 0)] = -np.pi / 2
        grid["slope_gamma"] = -grid["slope_gamma"]
+    elif grid["input_type"] is GridInputType.ELMER_XIOS:


Does this work?

It's generally a good idea to make sure that at least one of the cases in this chain of if-elif-statements is entered. I always append a final else: raise Exception(f"Cannot handle unexpected {grid["input_type"]=}"). This will make sure that the program exits.

BenjaminRodenberg · 2025-11-25T19:06:14Z

I'm taking care of some of the changes suggested in my review here: #44

I will merge #44 first, then integrate the updated main into this PR. This should resolve most of my comments above.

BenjaminRodenberg · 2025-11-25T19:48:01Z

-            io, OUTFILE = LOOP_write_to_file.main(OUTFILE, io, OUT, grid, t, time2, C)
-            pass
+            if grid["input_type"] is GridInputType.MATLAB:
+                gridtype = "structured"


To make sure: gridtype = "unstructured" is not tested/supported yet? Or do you manually set gridtype = "unstructured" during development here?

BenjaminRodenberg · 2025-11-25T19:59:41Z

        grid["slope_gamma"][(grid["slope_x"] < 0) & (grid["slope_y"] == 0)] = -np.pi / 2
        grid["slope_gamma"] = -grid["slope_gamma"]
+    elif grid["input_type"] is GridInputType.ELMER_XIOS:
+        grid = read_elmer_xios_grid(grid=grid, gridfile=args.elmer_xios_mesh)


Suggested change

grid = read_elmer_xios_grid(grid=grid, gridfile=args.elmer_xios_mesh)

grid = read_elmer_xios_grid(grid=grid, gridfile=args.netcdf_mesh_unstructured)

args.elmer_xios_mesh does not exist. Can you clarify?

BenjaminRodenberg · 2025-11-26T16:05:45Z

I performed another merge of main into this PR. I will push in a moment. Note that there is a good chance that the merge broke something. This should not be dramatic but just in case: The latest known working commit of this PR is 0b14bf3.

BenjaminRodenberg

Some minor comments on the design. I can take care of this as soon as you are done. Please let me know when I can take over here again.

BenjaminRodenberg · 2025-12-02T19:16:23Z

-        help="End time of the simulation in format 'DD-Mon-YYYY HH:MM', e.g., '02-Jan-1979 09:00'.",
-        default="2-Jan-1979 09:00",
+        help="End time of the simulation in format 'DD-Mon-YYYY HH:MM'",
+        default="2-Jan-1979 00:00",


I did another merge of main into this PR. The default values are quite arbitrary. So I'm fine to just change the default to the value you are using if this saves you some typing.

You can also use the new options to provide different values.

BenjaminRodenberg

Looks good to me.

ClemensSchannwell · 2025-12-03T10:22:52Z

Yes, I have tested the new version this morning and see no regression. So also from my side it looks good to go!

* Comment out grid assignment from IN['h'] * Fix regression of main w.r.t baseline by reverting some changes from #12 and #57.

ClemensSchannwell and others added 8 commits October 20, 2025 15:37

read unstructured elmer-xios grid

1ed8370

add elmer_xios type

1e6dff8

add elmer-xios-mesh option

7890ea8

allow output writing for unstructured grids

31384f8

restructure code for reading surface geometry from elmer-xios output

cbb2c5c

clean up print statements

e6a2fc2

Merge branch 'main' into feature_greenland

f5aea8b

Fix formatting

1833131

BenjaminRodenberg reviewed Nov 6, 2025

View reviewed changes

Comment thread src/dummy.py Outdated

BenjaminRodenberg mentioned this pull request Nov 6, 2025

Add minimal CONTRIBUTING.txt with guidelines #16

Closed

BenjaminRodenberg reviewed Nov 7, 2025

View reviewed changes

Comment thread src/dummy.py Outdated

ClemensSchannwell and others added 6 commits November 11, 2025 07:50

avoid using surface topography from elmer

3cb21e9

adjust runtime that it matches with elmer

4be61a8

fix merge conflicts

db825d1

fix formatting

29350ad

Rename (for safer merge).

dcea3f7

Merge branch 'main' into feature_greenland

ea7e28c

BenjaminRodenberg added this to the v0.2.0 milestone Nov 20, 2025

BenjaminRodenberg requested changes Nov 25, 2025

View reviewed changes

BenjaminRodenberg added 3 commits November 25, 2025 12:37

Add minimal EBFMCouplingConfig class.

1428593

Merge branch 'main' into minimal-coupling-configuration

07f050f

Introduce GridConfig and general refactoring.

e163957

BenjaminRodenberg added 3 commits November 25, 2025 20:21

Simplify and check for shading.

b6c1b0b

Add pointers to #11.

2cf7b25

Merge branch 'main' into feature_greenland

0b14bf3

BenjaminRodenberg reviewed Nov 25, 2025

View reviewed changes

Merge branch 'minimal-coupling-configuration' into feature_greenland

3b37aee

BenjaminRodenberg reviewed Nov 25, 2025

View reviewed changes

BenjaminRodenberg added 3 commits November 25, 2025 21:00

Apply some changes suggested in review.

9b044b9

Merge branch 'main' into feature_greenland

c404755

Check for property dem_file instead of grid type.

f222ff9

Set dem_file=None by default.

a859699

BenjaminRodenberg reviewed Dec 1, 2025

View reviewed changes

Comment thread src/ebfm/grid.py Outdated

Rename XIOS_CUSTOM to ELMERXIOS.

c495e6e

BenjaminRodenberg reviewed Dec 2, 2025

View reviewed changes

Comment thread src/ebfm/config.py

Comment thread src/ebfm/config.py

Comment thread src/ebfm/INIT.py

BenjaminRodenberg added 2 commits December 2, 2025 20:05

Merge branch 'main' into feature_greenland

631b8f6

Change default value.

2101e63

BenjaminRodenberg reviewed Dec 2, 2025

View reviewed changes

BenjaminRodenberg added 3 commits December 2, 2025 20:27

Store structured/unstructured in grid.

01229e7

Reduce code duplication.

e7d2807

Reduce code duplication and restructure.

db2c652

BenjaminRodenberg force-pushed the feature_greenland branch from b8e6e52 to db2c652 Compare December 2, 2025 20:11

Cleanup.

48f8008

BenjaminRodenberg approved these changes Dec 3, 2025

View reviewed changes

Add Changelog entry.

67f59f5

BenjaminRodenberg merged commit b473704 into main Dec 3, 2025
6 checks passed

BenjaminRodenberg deleted the feature_greenland branch December 3, 2025 11:07

BenjaminRodenberg mentioned this pull request Dec 12, 2025

Remove unnecessary read_elmer_xios_grid #57

Merged

1 task done

BenjaminRodenberg mentioned this pull request Jan 19, 2026

Add work-arounds required for coupled run #53

Merged

1 of 2 tasks

BenjaminRodenberg added a commit that referenced this pull request Jan 19, 2026

Add work-arounds required for coupled run (#53)

f2cd676

* Comment out grid assignment from IN['h'] * Fix regression of main w.r.t baseline by reverting some changes from #12 and #57.

BenjaminRodenberg mentioned this pull request Oct 20, 2026

Add an example for --netcdf-dem-mesh-unstructured (Elmer/Ice XIOS output) #196

Closed



		def main(OUTFILE, io, OUT, grid, t, time, C):
		def main(OUTFILE, io, OUT, grid, t, time, C, gridtype):

	grid = read_elmer_xios_grid(grid=grid, gridfile=args.elmer_xios_mesh)
	grid = read_elmer_xios_grid(grid=grid, gridfile=args.netcdf_mesh_unstructured)

Conversation

ClemensSchannwell commented Oct 23, 2025 • edited by BenjaminRodenberg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenjaminRodenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BenjaminRodenberg commented Nov 14, 2025

Uh oh!

BenjaminRodenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BenjaminRodenberg Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg commented Nov 25, 2025

Uh oh!

BenjaminRodenberg Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg commented Nov 26, 2025

Uh oh!

Uh oh!

BenjaminRodenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg left a comment

Choose a reason for hiding this comment

Uh oh!

ClemensSchannwell commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ClemensSchannwell commented Oct 23, 2025 •

edited by BenjaminRodenberg

Loading

BenjaminRodenberg Nov 25, 2025 •

edited

Loading

BenjaminRodenberg Nov 25, 2025 •

edited

Loading