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.

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

Pass mk.MeshKernel instead of mk.mesh2d to xu.Ugrid2d.from_meshkernel #188

Open
veenstrajelmer opened this issue Dec 5, 2023 · 0 comments

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Dec 5, 2023

This way it is possible to derive the projected bool automatically, since it is set in the mk.MeshKernel instance (albeit indirectly via get_projection(). It would avoid the need to derive the projected bool from the mk.MeshKernel instance before converting the mesh2d to Ugrid2d and limits the amount of input arguments.

E.g. in dfm_tools we derive projected from the mk.MeshKernel instance before passing it to from_meshkernel:
https://github.com/Deltares/dfm_tools/blob/9a527af6f0df6529aecf46d434315bb277158797/dfm_tools/xugrid_helpers.py#L550-L557
In Deltares/dfm_tools#685 we moved to deriving crs and projected from uds instead of from mk.MeshKernel

And in hydromt_delft3dfm we pass projected and crs, altough projected is derived from crs before passing it to from_meshkernel:
https://github.com/Deltares/hydromt_delft3dfm/blob/a97d2b491501e1fc78042e58292405e8bbcd2baa/hydromt_delft3dfm/mesh_utils.py#L259-L267

After implementation, it is still important to check if the passed crs and mk.MeshKernel are both geographic or both projected. But that is slighly better than the current situation, since we should actually check whether crs and mk.MeshKernel and projected are aligned.

The above also goes for mesh1d/Ugrid1d.

Alternatively
Alternatively, derive projected from crs. This would avoid the need to pass a different meshkernel instance. In that case it might be easiest to pick it up in #187

More info
Another consideration is consistency. We have back-forth conversions for meshkernel with meshkernel and from_meshkernel. These functions are available for UgridBase, Ugrid1d and Ugrid2d. meshkernel always creates a MeshKernel() instance, but from_meshkernel expects a mesh2d or mesh1d. So for consistency it would make sense to go for the first approach (and check the geographic property of the meshkernel instance against that of the passed crs).

@veenstrajelmer veenstrajelmer changed the title use mk.MeshKernel instead of mk.mesh2d in xu.Ugrid2d.from_meshkernel Pass mk.MeshKernel instead of mk.mesh2d to xu.Ugrid2d.from_meshkernel Dec 5, 2023
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

No branches or pull requests

1 participant