-
Notifications
You must be signed in to change notification settings - Fork 4
UNST-9376: Accept both "waterlevel" and "waterlevelbnd" #369
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
base: main
Are you sure you want to change the base?
Conversation
ec_typedefs::tEcNetCDF: Register what variables are time varying ec_netcdf_timeseries.f90::ecNetCDFInit: - Fill array time_varying_var - Vertical layering should be identified with "axis=Z" instead of "positive = notEmpty" - Vertical layering: accept standard_names "z, zcoordinate, zcoordinate_c", exclude "zcoordinate_w, zcoordinate_wu" ec_netcdf_timeseries.f90::ecNetCDFScan: - ltl is defined by calling len_trim_nobnd ec_netcdf_timeseries.f90::len_trim_nobnd: - New function, returning the length of a string excluding "bnd" at the end
|
@arthurvd : At some point we need to discuss how to facilitate 3D quantities for nesting, including z-coordinates varying in time. This PR is a first step, enabling Theo to continue. |
When reading "zcoordinate_c"-related info, the nesting functionality is ruined. TODO: find out why and implement a proper fix.
arthurvd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adri and Arthur will followup with some improved design under UNST-9376 first.
| if (ncptr%vptyp < 1) then | ||
| call setECMessage("ec_bcreader::ecNetCDFCreate: Unable to determine vertical coordinate system.") | ||
| ! This line does not work: if (any((/ 'z ', 'zcoordinate ', 'zcoordinate_c'/) == ncptr%standard_names(iVars))) then | ||
| if (any((/ 'z ', 'zcoordinate '/) == ncptr%standard_names(iVars))) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to do this differently, let's refine a proper 3D initialization under the other dedicated issue: UNST-8909 (so not do this here)
| if (ncptr%vptyp < 1) then | ||
| call setECMessage("ec_bcreader::ecNetCDFCreate: Unable to determine vertical coordinate system.") | ||
| ! This line does not work: if (any((/ 'z ', 'zcoordinate ', 'zcoordinate_c'/) == ncptr%standard_names(iVars))) then | ||
| if (any((/ 'z ', 'zcoordinate '/) == ncptr%standard_names(iVars))) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p.s. Coding guidelines require the modern format for initialization arrays, namely: [ .. ] instead of (/ .. /).
| if (ncptr%nLayer > size(ncptr%vp)) then | ||
| call realloc (ncptr%vp, ncptr%nLayer, stat=ierr, keepExisting=.true.) | ||
| end if | ||
| if (ierr /= 0) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If 3D work is postponed until UNST-8909, why not simply have an if .. cycle here?
| if (ncptr%nLayer > size(ncptr%vp)) then | |
| call realloc (ncptr%vp, ncptr%nLayer, stat=ierr, keepExisting=.true.) | |
| end if | |
| if (ierr /= 0) return | |
| allocate (ncptr%vp(ncptr%nLayer), stat=ierr) | |
| if (ierr /= 0) cycle |
| do ivar = 1, ncptr%nVars | ||
| ltl = len_trim(quantity) | ||
| if (strcmpi(trim(ncptr%standard_names(ivar)), trim(quantity))) exit | ||
| if (strcmpi(trim(ncptr%standard_names(ivar)), trim(quantity), ltl)) exit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would hope we can make this work instead using the existing ncstdnames approach, by having something like this in ecProviderCreateNetCdfItems():
case ('waterlevelbnd')
ncvarnames(1) = 'waterlevel'
ncstdnames(1) = 'sea_surface_height'
Let's refine this together under UNST-9376 for proper followup in next sprint.
Commit Description
ec_typedefs::tEcNetCDF: Register what variables are time varying
ec_netcdf_timeseries.f90::ecNetCDFInit:
ec_netcdf_timeseries.f90::ecNetCDFScan:
ec_netcdf_timeseries.f90::len_trim_nobnd:
What was done
TODO
Evidence of the work done
<add video/figures if applicable>
Tests
<add testcase numbers if applicable, Issue number>
Documentation
<add description of changes if applicable, Issue number>
Issue link
UNST-9376