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

[utils] netCDF converter #2654

Merged
merged 30 commits into from Dec 9, 2019
Merged

[utils] netCDF converter #2654

merged 30 commits into from Dec 9, 2019

Conversation

@rinkk
Copy link
Member

rinkk commented Sep 10, 2019

Command line utility to conver netCDF-files into OGS meshes. Works for both 2d and 3d meshes + time. Multiple time steps can be saved as seperate meshes or as one mesh with one scalar array per time step.

(A follow-up PR will move all non-ui functionality into FileIO and adjust the DataExplorer interface to use that as well.)

  1. Feature description was added to the changelog

Please note:

  • Conan minimum version was bumped to 1.20, do pip install --upgrade conan!
  • Updated Conan Qt package to 5.13.2
@rinkk rinkk force-pushed the rinkk:NetCdfSplit branch from bfa1038 to 3fb6a75 Sep 11, 2019
@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 11, 2019

Is there a test for the command line tool?

@rinkk

This comment has been minimized.

Copy link
Member Author

rinkk commented Sep 12, 2019

No, there isn't. The smallest nc-file I have is ~170 mb. From what I understand, this is too large for our tests?

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 12, 2019

That's too large. There must be small files. As usual, the reason is to have working examples, s.t. in case of code changes the utility is covered.

std::array<std::size_t, 4> const& dim_idx_map,
bool is_time_dep)
{
std::size_t const temp_offset = (is_time_dep) ? 1 : 0;

This comment has been minimized.

Copy link
@endJunction

endJunction Sep 12, 2019

Member

temp means temperature?
why no simply offset?

This comment has been minimized.

Copy link
@rinkk

rinkk Sep 23, 2019

Author Member

temporal offset ... in contrast to the spatial offset checked further on.

This comment has been minimized.

Copy link
@endJunction

endJunction Sep 23, 2019

Member

I'd rename the variable for readability.

This comment has been minimized.

Copy link
@rinkk

rinkk Sep 23, 2019

Author Member

If it's the same to you I'll keep it as it is because the UI explicitely says "temporal dimension".

This comment has been minimized.

Copy link
@endJunction

endJunction Sep 23, 2019

Member
Suggested change
std::size_t const temp_offset = (is_time_dep) ? 1 : 0;
std::size_t const temporal_offset = (is_time_dep) ? 1 : 0;
Copy link
Member

endJunction left a comment

The converter looks to be very specific for a given input type and relies on series of inputs from the user.
It is not a generic netcdf converter/extractor, so I'd recommend to rename it to the specific actions it does for that particular file format.

@rinkk

This comment has been minimized.

Copy link
Member Author

rinkk commented Sep 23, 2019

What do you mean by "very specific for a given input type"? It uses arbitrary 2D/3D/4D rasters (gives an error msg for 1D) and creates a mesh out of these. That's the data netCDF contains. I could add a conversion for 1D, too, but it wouldn't make much sense since that is usually diagrams from my experience.

What do you think is specific about that? What other data does netCDF contain that you are aware of? In what sense is it not generic?

@rinkk

This comment has been minimized.

Copy link
Member Author

rinkk commented Sep 23, 2019

The smallest nc-file I have is ~170 mb. From what I understand, this is too large for our tests?

That's too large. There must be small files. As usual, the reason is to have working examples, s.t. in case of code changes the utility is covered.

I understand but don't have a small file.

Unidata offers a number of test files ... I've seen a 22 mb test example for surface data. Would that be acceptable? There might be smaller files but I'd have to check it's not just 1D data. There's also the license issue ... Unidata software is offered under 3 clause BSD but there's no mention if the same rules apply for their example data sets. What's your opinion on that?

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 23, 2019

https://www.unidata.ucar.edu/software/netcdf/examples/files.html has several files smaller than 10MiB, e.g. the testrh.nc (only 90KiB).

@rinkk

This comment has been minimized.

Copy link
Member Author

rinkk commented Sep 23, 2019

Yes, but I need a file containing a 2D, 3D, or 4D array. That is why I said I'd have to check for a small file with a 2D array assuming you think the license automatically extends to datasets as well.

@rinkk rinkk force-pushed the rinkk:NetCdfSplit branch from 4e195bb to c742cb4 Sep 23, 2019
@bilke bilke force-pushed the rinkk:NetCdfSplit branch from 2789517 to 6843812 Dec 5, 2019
@bilke bilke force-pushed the rinkk:NetCdfSplit branch from 67007b8 to cd8ad70 Dec 5, 2019
@bilke bilke force-pushed the rinkk:NetCdfSplit branch from 1447c24 to 9c45a1e Dec 6, 2019
@bilke

This comment has been minimized.

Copy link
Member

bilke commented Dec 6, 2019

@endJunction @rinkk Ready to merge, everything is green!

@bilke bilke merged commit f0e7c8a into ufz:master Dec 9, 2019
2 of 3 checks passed
2 of 3 checks passed
ufz.ogs #20191206.9 failed
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
@bilke bilke deleted the rinkk:NetCdfSplit branch Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.