Fm/feature/unst 8910 nesting read 3d vectors3 by mihai-pricope · Pull Request #1019 · Deltares/Delft3D

mihai-pricope · 2026-06-30T07:55:09Z

What was done

Read quantities with time interpolated Z-Dimension from _his.nc files (_his.nc files are produced by dflowfm, so this will enable nesting).

Evidence of the work done

Video/figures
<add video/figures if applicable>
Clear from the issue description
Not applicable

Tests

Tests updated
<add testcase numbers if applicable, Issue number>
Not applicable

Documentation

Documentation updated
<add description of changes if applicable, Issue number>
Not applicable

Issue link

https://issuetracker.deltares.nl/browse/UNST-8910

we want to detect Z based on coordidate dimensionalities

arthurvd

Hi Mihai, a few small remarks as well as a few high level questions. Have a look at them, and we can have a chat to discuss some of them if you want.

arthurvd · 2026-06-30T13:33:53Z

+
+            do idIdx = 1, size(sourceItemIds)
+               sourceItemId = sourceItemIds(idIdx)
+               success = ecAddConnectionSourceItem(ecInstancePtr, connectionId, sourceItemId)


catch error, such that subsequent iterations cannot hide any errors:

Suggested change

success = ecAddConnectionSourceItem(ecInstancePtr, connectionId, sourceItemId)

success = ecAddConnectionSourceItem(ecInstancePtr, connectionId, sourceItemId)

if (.not. success) then

exit

end if

arthurvd · 2026-06-30T13:57:56Z

      integer :: ncid !< unique NetCDF ncid
      character(len=maxFileNameLen) :: ncfilename !< netCDF filename
      integer, allocatable, dimension(:) :: dimlen !< lengths of dimensions
+      integer, allocatable, dimension(:) :: variable_dimension !< lengths of dimensions


Rename to variable_ndims (recognized NetCDF concept), or alternatively variable_rank.

(Current name is ambiguous whether rank/shape/dimension ids are stored.)

arthurvd · 2026-06-30T14:21:47Z

      integer, intent(in) :: l_id
      integer, dimension(:), intent(in) :: dims
      integer, intent(in) :: timelevel
+      integer, intent(in) :: func


Move this to last dummy arg declaration line (should be in order of appearance).
And nice to have docstring (should be a BC_FUNC_* enum value that specifies the type of timeseries.)

arthurvd · 2026-07-01T06:44:22Z

+            if (.not. ecItemSetType(instancePtr, zTargetItemId, itemPT%accessType)) then
+               return
+            end if
+            if (.not. ecItemSetQuantity(instancePtr, zTargetItemId, itemPT%quantityPtr%id)) then


Why does the zTargetItem get the same id as the actual data item (e.g. 'salinity'). Is this required for correct workings? (I'd expect each item to have a descriptive id, so something describe the vertical z coordinate_.

Is bcBlockPtr%ncptr%variable_names(bcBlockPtr%ncptr%vertical_coordinate_id) more appropriate?

I made some modification but it seems that if the item is not called polytim_item, is not gonna work.
zQuantityId = ecInstanceCreateQuantity(instancePtr)
if (.not. (ecQuantitySet(instancePtr, zQuantityId, name='polytim_item'))) then
return
end if
need to debug further to figure out why

arthurvd · 2026-07-01T06:54:02Z

+         ! ToDo, more generic approach to determine veriabel name from type of boundary
+         call str_lower(quantityname)
+         if (index(trim(bctfilename)//'|', '_his.nc|') > 0) then
+            ! History file
+            if (strcmpi(quantityname,'waterlevelbnd'           )) quantityname = 'waterlevel'
+            if (strcmpi(quantityname,'salinitybnd'             )) quantityname = 'salinity'
+            if (strcmpi(quantityname,'temperaturebnd'          )) quantityname = 'temperature'
+            if (strcmpi(quantityname,'uxuyadvectionvelocitybnd')) quantityname = 'x_velocity'
+         else
+            ! Old existing nc files 
+            if (strcmpi(quantityname,'waterlevelbnd'           )) quantityname = 'waterlevelbnd'
+            if (strcmpi(quantityname,'salinitybnd'             )) quantityname = 'so'
+            if (strcmpi(quantityname,'temperaturebnd'          )) quantityname = 'thetao'
+            if (strcmpi(quantityname,'uxuyadvectionvelocitybnd')) quantityname = 'ux'
+            ! Old existing nc files (temporary fix for Tom, durban)
+            ! if (strcmpi(quantityname,'so'                      )) quantityname = 'salinity'
+            ! if (strcmpi(quantityname,'thetao'                  )) quantityname = 'temperature'
+         end if


Could you not use Thomas's utility function here? --> m_ec_support::ecSupportNetcdfGetQuantityCandidateNames()

extend the waterlevelbnd case to include a fallback name

add salinity and other bnds

Where needed, call ecProviderSearchStdOrVarnames(fileReaderPtr, i, idvar, ncstdnames, ncvarnames, uservarnames=nccustomnames) and same for fallback names.

(I'm ok if you create a separate issue for this to do it in a future PR.)

Added this. https://issuetracker.deltares.nl/browse/UNST-10104
I have also a few ideas around it, but I think at this point this can easily scoped out of this PR

arthurvd · 2026-07-01T07:10:40Z

+         ! Only interpolate if both T0 and T1 contain at least 1 valid value, else set valuesT to dmiss
+         if (all(valuesT0 == dmiss) .or.  all(valuesT1 == dmiss)) then
+             valuesT = dmiss
         else


This does not handle the situation where most values are not dmiss, but on a particular index i both valuesT0 and valuesT1 have a dmiss. So then we fall through to the do-loop on line 1246, which starts to do math on line 1255. Should you not catch the double dmiss there (at the index i level).

arthurvd · 2026-07-01T07:11:53Z

+            ! TK_Temp: Deal with one sided interpolation, set kL or kR to 0 if arr1D does not contain valid values
+            ! TK_Temp: Left side


Suggested change

! TK_Temp: Deal with one sided interpolation, set kL or kR to 0 if arr1D does not contain valid values

! TK_Temp: Left side

! Deal with one sided interpolation, set kL or kR to 0 if arr1D does not contain valid values at the current time.

! Left side:

arthurvd · 2026-07-01T07:15:28Z

            select case (connection%converterPtr%operandType)
            case (operand_replace_element, operand_replace, operand_add)


Warning: you will probably encounter merge conflicts here with changes from Thomas from PR #959. Try to adopt his new apply_operand() approach in your code fragment here.

arthurvd · 2026-07-01T07:17:01Z

+            if (kL > 0) then
+               kbeginL = vectormax * maxlay_src * (kL - 1) +  1! refers to source right column
+               kendL   = kbeginL + vectormax*maxlay_src - 1
+               if (all(connection%sourceItemsPtr(1)%ptr%targetFieldPtr%arr1Dptr(kbeginL:kendL) == dmiss)) then


The code here (and below too for kR) enables one-sided interpolation if ALL layer values are dmiss in the kL point. But what if only some layers contain dmiss on point kL: will those dmiss values end up in a linear interpolation below, or not?

arthurvd · 2026-07-01T07:22:58Z

+         block
+            integer :: zItemId
+
+            if (present(zTargetItemId) .and. present(zFileReaderId)) then


High-level question about the code below: here you're creating a subconverter + connection for the extra z quantity. How does that relate to the new code in ec_addtimespacerelation(), where also some extra association is made between multiple source items and a single target item (code over there changed ecFindItemInFileReader() --> ecFindItemsInFileReader() and runs over do idIdx = 1, size(sourceItemIds).

…re/UNST-8910_nesting_read_3d_vectors3

…nesting_read_3d_vectors3

mihai-pricope added 25 commits June 15, 2026 14:34

initial import from Theo's branch

22b12e4

UNST-9839: Integration Test for 3D scalar & vector case

667c6a9

Something is working on One Pass for Z-Interpolation

4b901f7

simplify stuff

5939f29

adding zSource

e7cb37e

some cleaning

bc85801

eliminate arrz

961b590

fixed initialization

e9afe7b

remove ncType

3076fbb

we want to detect Z based on coordidate dimensionalities

detect if z is time varying in netcdf

e7ab08e

fixing test crashes

6bb31ad

test results for win64

c3e9110

renamings + ext force fix

9406969

fix typo

3dea205

more cleanup

613405f

fix z coord for non time varying

36f2b31

move nesting tests to validation

44c7fec

fix xml

f173a43

remove possible unnecessary block

612fa67

style changes

a4fe229

fix build

ebff44e

more cleaning

78ab464

removing old test xmls

4b22d2a

more fixes

f112ec5

UNST-8910: extra writing fixes for z layers

10ffc19

arthurvd requested changes Jul 1, 2026

View reviewed changes

mihai-pricope added 4 commits July 2, 2026 11:58

Merge commit '33c17ce08961dd5457b30c00a21d13147399eaf0' into fm/featu…

37ba09c

…re/UNST-8910_nesting_read_3d_vectors3

Merge commit '5e2eb1ab1010bda93865b5edfd0daa1ed0d51f3f' into fm/featu…

2e90bd7

…re/UNST-8910_nesting_read_3d_vectors3

fixing build

fff62bb

finalize merge

f4139ea

mihai-pricope force-pushed the fm/feature/UNST-8910_nesting_read_3d_vectors3 branch from 70aa590 to f4139ea Compare July 3, 2026 06:37

mihai-pricope added 3 commits July 3, 2026 09:51

review fixes

1b078cd

more review changes

bc13624

Merge remote-tracking branch 'origin/main' into fm/feature/UNST-8910_…

608011b

…nesting_read_3d_vectors3

		! TK_Temp: Deal with one sided interpolation, set kL or kR to 0 if arr1D does not contain valid values
		! TK_Temp: Left side

		select case (connection%converterPtr%operandType)
		case (operand_replace_element, operand_replace, operand_add)

Uh oh!

Conversation

mihai-pricope commented Jun 30, 2026

What was done

Evidence of the work done

Tests

Documentation

Issue link

Uh oh!

arthurvd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants