t3D header not correct yet

[veenstrajelmer]

Describe the bug
Some issues with t3D header:
Missing:

• vertical position 1/2/3/n is not supported, should be added to QuantityUnitPair (or new object)
• timeInterpolation not written to file for T3D blocks
• verticalPositionType "ZDatum" but is not available in VerticalPositionType class
• verticalPositionType "ZSurf" is missing (newly added to this issue, Sep 29 '22)
  Inconsistencies:
• "verticalPositions" is written, while "Vertical position specification" is expected (in 2D3D manual). Does both work?
  Answer: in kernel: yes, in GUI: no. And we should move to 1 consistent keyword. Decision (Oct 13, 2022): support only the latest kernel keywords in the model fields + support spaced old keywords for compatibility via a pre root_validators, e.g., "Vertical position specification"
  (Pydantic already turns spaced fields into underscore fields, e.g., "vertical_position_specification")
  Background for D-HYDRO developers, see internal issue UNST-5801.
• "verticalPositionType " is written, while "Vertical position type" is expected (in 2D3D manual). Does both work?
  Answer: same as previous one. Support "vertPositionType" and backwards compability for "Vertical position type".

In summary, these are the correct keywords:

• timeInterpolation
• vertPositionType
• vertPositions
• vertInterpolation
• vertPositionIndex

To Reproduce

from pathlib import Path
import numpy as np

from hydrolib.core.io.bc.models import (
    ForcingModel,
    QuantityUnitPair,
    TimeInterpolation,
    T3D,
    VerticalPositionType,
)

bc_south = ForcingModel()
depth_list = [-2000,-1000,-500,-100]
times_list = np.array([0,60,120,180,240,300,360,420])
datablock = np.random.uniform(low=-40, high=130.3, size=(len(times_list),len(depth_list)))
datablock_incltime = np.concatenate([times_list[:,np.newaxis],datablock],axis=1)
list_QUP_perlayer = [QuantityUnitPair(quantity='salinity', unit='1e-3') for iL in range(len(depth_list))] #TODO REPORT: verticalposition 1/2/3/n is not supported
ts_one = T3D(name='3d_bnd_0001',
             verticalpositions=depth_list, #TODO: should be "Vertical position specification = [..]" but is verticalPositions = [..]" (both possible?)
             verticalInterpolation='linear',
             verticalPositionType=VerticalPositionType('ZBed'), #TODO: should be "Vertical position type = zdatum" but is "verticalPositionType = ZBed" (zdatum is niet beschikbaar)
             quantityunitpair=[QuantityUnitPair(quantity="time", unit="seconds since 2022-01-01 00:00:00 +00:00")]+list_QUP_perlayer,
             timeinterpolation=TimeInterpolation.linear, #TODO: not passed on to bc file
             datablock=datablock_incltime.tolist(), 
             )
bc_south.forcing.append(ts_one)
bc_south.save(filepath=Path("steric_south2.bc"))

Expected behavior
described in description

Version info (please complete the following information):

• OS: Windows, Anaconda, Spyder
• Version: 0.3.0

[tim-vd-aardweg]

Discussed the issue with @arthurvd:

• This issue was created on 19th of August. Support for ZDatum was added in Add support for ZDatum for Vertical Position Type #323 after the 19th of August. So that should work properly now.
• While discussing the verticalPositionType we noticed the ZSurf option was missing. The title of this issue is broad enough (and the fix small enough) that we can actually add the ZSurf option in this issue.
• Arthur will dig into the kernel code to see if we can/should use verticalPositions or Vertical Position Specification.
• We will add timeInterpolation keyword to T3D class.
• We will add verticalPosition to the datablock of T3D and add appropriate validation to check if the integer given here is not out of range (i.e. should be lower than the length of the verticalPositionSpecification array).

[arthurvd]

@tim-vd-aardweg : I've update the original issue description at the top.
Note the correct naming: "Vertical position specification", "vertical position", "Vertical position specification".

[tim-vd-aardweg]

@arthurvd Thanks for checking this. So we basically have to adhere to the 2D3D manual for the .bc file then? So we also have to change timeInterpolation (which is the current implementation in Hydrolib) to Time Interpolation?

[arthurvd]

@arthurvd Thanks for checking this. So we basically have to adhere to the 2D3D manual for the .bc file then? So we also have to change timeInterpolation (which is the current implementation in Hydrolib) to Time Interpolation?

No, let's keep that at the new "timeInterpolation" keyword already. I am preparing an issue where we make kernel+GUI consistent again with all new lowerCamelCase namings, and timeInterpolation is now already supported by both kernel and GUI.

[tim-vd-aardweg]

Ok!

[arthurvd]

@tim-vd-aardweg: and, when adding/changing some of these fields, could you also add a docstring for each of them? For example:

    timeinterpolation: TimeInterpolation = Field(alias="timeInterpolation")
    """TimeInterpolation: Type of time interpolation. See enum type for supported values."""

[tim-vd-aardweg]

I will