Skip to content

Conversation

@ChristianSteger
Copy link
Collaborator

@ChristianSteger ChristianSteger commented Jul 1, 2025

Enables the usage of Copernicus DEM (30 m resolution) as input topography data for EXTPAR.

https://dataspace.copernicus.eu/explore-data/data-collections/copernicus-contributing-missions/collections-description/COP-DEM

Copernicus DEM is a high-resolution (30 m) topography data set, which is based on recent satellite data. Its superior accuracy compared to other DEMs was demonstrated in various publications (e.g., Guth (2021)). It is particular useful as a replacement for ASTER data for high-resolution simulations (~1km grid spacing and smaller) and for regions with complex terrain (ASTER data reveals, for instance, some distinctive artifacts in the Alps - particularly at steep north-facing slopes). Another benefit compared to ASTER is its global coverage.

The added run script (last commit) was used for testing - I'm not sure if it is necessary to merge it as run scripts will no longer be maintained at some point in the near future...

References:
Guth, P. L. and Geoffroy, T. M. (2021). LiDAR point cloud and ICESat-2 evaluation of 1 second global digital elevation models: Copernicus wins. Transactions in GIS, 25, 2245–2261.

@ChristianSteger
Copy link
Collaborator Author

One point could be optimised/simplified in the future (if needed & resources are available): Copernicus DEM data comes with a reduced longitudinal resolution towards higher latitudes (see https://dataspace.copernicus.eu/sites/default/files/media/files/2024-06/geo1988-copernicusdem-spe-002_producthandbook_i5.0.pdf). Currently, this data is interpolated to a latitude-longitude grid with uniform spacing of 1/3600 degrees so that the data can be processes in EXTPAR analogous to GLOBE, ASTER and MERIT data. However, when topography data is remapped to the ICON grid in EXTPAR, elevation data of longitudinal rows is aggregated before remapping. This unnecessary back and forth interpolation could be eliminated, which would also result in a considerable smaller memory usage of the Copernicus DEM...

Comment on lines 407 to 408
ndata(ie,je,ke) = ndata(ie,je,ke) + 1
sgsl(ie,je,ke) = sgsl(ie,je,ke) + sl(i)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ndata(ie,je,ke) = ndata(ie,je,ke) + 1
sgsl(ie,je,ke) = sgsl(ie,je,ke) + sl(i)
ndata(ie,je,ke) = ndata(ie,je,ke) + 1
sgsl(ie,je,ke) = sgsl(ie,je,ke) + sl(i)

Also, could you do me a favor and apply the same changes to the cases above? 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

h22(ie,je,ke) = h22(ie,je,ke) + dhdydy(i)
ENDIF
ENDIF
CASE(topo_copernicus)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless you have a reason to add this code here I think we could just skip it, since COSMO is not really used anymore and I will probably start to remove COSMO related code at some point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done - I removed the implementation of Copernicus DEM for COSMO.

half_gridp = 1./(3600.*2.) ! the resolution of the ASTER data is 1./3600. degrees as it is half a grid point
! it is additionally divided by 2
CASE (topo_gl) ! GLOBE topography is composed of 16 tiles
half_gridp = 1./(3600.*2.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove these comments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought the comment is unnecessary because it's easy inferable from the code what is done here. But I guess it's also fine to leave it (I reintroduced it in a more concise way).

tiles_lon_max(i) = REAL(NINT(tiles_lon_max(i) + half_gridp)) !< added, as the ASTER/GLOBE/MERIT data
tiles_lat_min(i) = REAL(NINT(tiles_lat_min(i) + half_gridp)) !< is located at the pixel center
tiles_lat_max(i) = REAL(NINT(tiles_lat_max(i) - half_gridp))
SELECT CASE (itopo_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this already part of PR #426?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's actually merge that one first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's true. But this specific modification is also needed for Copernicus DEM to run correctly. If one would merge the current master (including the branch grid_shift_topo) into this branch, the single difference should be the line
CASE(topo_merit, topo_copernicus) ! edges of raw data tiles do not align with integer coordinates

@stelliom
Copy link
Contributor

In general, I think now you are only missing the new tests for Copernicus (if you want to also add the ICON ones for ASTER we can do it in a separate PR) and the update to the docs.

I recently added a new PR template, maybe this can be useful for you as well. I will post it here, since this PR was opened before the template was made.

@stelliom
Copy link
Contributor

stelliom commented Jul 23, 2025

Workflow for merging PRs

Please read these instructions carefully and follow the steps below before requesting a review by the maintainers. This way we can ensure a smoother review process and your changes will be merged sooner.

Additionally, if this is the first PR you open in EXTPAR make sure to read the "Information for EXTPAR Developers" section in the documentation.

Checklist

  • Provide a detailed description of your changes.
  • If you implemented a new feature:
    • Document it in the correct Markdown file(s) under the docs/ directory.
    • Add a new test or make sure your changes are already tested.
  • Your code follows the style guidelines.
  • Your changes only touch the files/lines relevant for you.
  • All four required checks pass (see "Testing and debugging" for more details).
  • No conflicts with the base branch.

If all the points above are satisfied you can ask for a review by selecting stelliom as a reviewer.

For any questions please ping @stelliom on this PR.

Testing and debugging

The most important test for PRs is the one labeled "EXTPAR Testsuite on Jenkins". This checks that the results of all testcases (described by the namelists in test/testsuite/data) did not change compared to the references.

You can launch the testsuite by writing launch jenkins as a comment in the PR that you want to test. Once completed, the result of the testsuite will be shown on the PR (failure or success).

If you need more details on the testsuite results (e.g., if you are trying to debug an error or you are simply unsure why the tests fail) you can launch the testsuite with launch jenkins(debug). This will run the tests as usual, but, once completed, you will be given a URL (via a comment on the PR) to access the logfiles and namelists of all tests that were run.

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.

3 participants