Skip to content

vertical aggregation#984

Open
otzi5300 wants to merge 13 commits into
mainfrom
aggregation_in_vertical
Open

vertical aggregation#984
otzi5300 wants to merge 13 commits into
mainfrom
aggregation_in_vertical

Conversation

@otzi5300

@otzi5300 otzi5300 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Added vertical mean, max, min

user can now do:

# In [1]:
fpath = Path("C:/Users/lajo/offlineSpace/tmp/dfs_testfiles/HD3D.dfsu")
dfs = mikeio.open(fpath)
ds = dfs.read(items=3)#, time=range(0,1000))

temp_vmean = ds[0].mean(axis="z")
temp_vmax = ds[0].max(axis="z")
temp_vmin = ds[0].min(axis="z")
# In [2]:
temp_vmean.plot()
image
# In [3]:
temp_vmax.plot()
image
# In [4]:
temp_vmin.plot()
image
# In [4]:
# Subset top 2 layers, take max
ds.sel(layers=[-1,-2]).max(axis="z")[0].plot()
image
# In [4]:
# Take vertical mean then max to get time, depth average
ds.mean(axis="z").mean(axis="time")[0].plot()
image

test

Two test files added:

On simple with 3 sigma and 2 z. No sigma stretching
On more complex with 3 sigma and 2 z. On second timesteps, surface level is increase and sigma streched. Bottom layer in element 2 is nan

Computation speed

the loop: for col_idx in range(n_cols) for sigma-z models is slow.

On a laptop, a mesh with:
number of nodes: 131263
number of elements: 192568
number of timesteps: 9504 records (396 days with hourly data)

takes around 1 minutes to do vertical mean on all-layers for one item

min and max takes only a few seconds

image

@otzi5300
otzi5300 requested a review from ecomodeller as a code owner June 3, 2026 13:56
@otzi5300 otzi5300 changed the title vertical aggregation with tests vertical aggregation Jun 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds vertical (layer-collapsing) aggregation support to the mikeio DataArray/Dataset APIs by introducing axis="z" for mean, min, and max on layered FM 3D geometries, returning a 2D result. It also extends FM 3D geometry reduction to explicitly support reducing over "z", and adds regression tests covering sigma-z behavior, dynamic zn, NaN/inactive layers, and layer subsetting.

Changes:

  • Add axis="z" handling for DataArray.mean/min/max and Dataset.mean/min/max (collapsing layered 3D to 2D; mean is thickness-weighted).
  • Extend GeometryFM3D.reduce() to support reducing over "z" and combinations with "element".
  • Add tests for dynamic sigma stretching, NaN/inactive layers, and layer selection/aggregation.

Reviewed changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
tests/test_dataset.py Adds dataset-level vertical aggregation tests (min/max/mean over "z"), including NaN/inactive layer coverage.
tests/test_dataarray.py Adds dataarray-level tests for thickness-weighted vertical mean with dynamic zn, NaN masking, and layer selection behavior.
src/mikeio/spatial/_FM_geometry_layered.py Adds GeometryFM3D.reduce() support for reducing over "z" (to 2D) and "z"+"element" (to 0D).
src/mikeio/dataset/_dataset.py Adds axis="z" dispatch for dataset min/max/mean to call the corresponding dataarray vertical operations.
src/mikeio/dataset/_dataarray.py Implements vertical mean/min/max for layered FM geometries via _mean_z() and _extremum_z().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mikeio/dataset/_dataarray.py
Comment thread src/mikeio/dataset/_dataarray.py
Comment thread src/mikeio/dataset/_dataarray.py
Comment thread src/mikeio/dataset/_dataarray.py
Comment thread src/mikeio/dataset/_dataarray.py
Comment thread src/mikeio/dataset/_dataarray.py Outdated
Comment thread src/mikeio/dataset/_dataarray.py Outdated
Comment thread src/mikeio/dataset/_dataarray.py Outdated
Comment thread src/mikeio/dataset/_dataarray.py
Comment thread tests/test_dataset.py Outdated
otzi5300 and others added 10 commits June 3, 2026 17:40
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 Move the per-element dynamic thickness calculation out of the column
 loop into a vectorized precomputation step (_compute_dynamic_dz).
 Previously, _compute_column_dz_dynamic was called for each of the
 column, resulting in large amount small numpy operations. Now dz is
 computed for all elements at once in timestep chunks (100 at a time)
 to limit memory usage, and the column loop just slices into the
 precomputed array — matching the performance pattern of max(axis="z")
 Replace the Python column loop in _extremum_z with a single
 np.fmin.reduceat / np.fmax.reduceat call. Since column elements
 are contiguous in memory, reduceat reduces each segment without
 any loop, padding, or intermediate arrays. Also pass np.fmin/fmax
 directly from the call sites instead of mapping from np.nanmin/nanmax.

speedup x3
@ecomodeller

Copy link
Copy Markdown
Member

@miab-dhi please take a look at this.

@miab-dhi

miab-dhi commented Jun 5, 2026 via email

Copy link
Copy Markdown
Contributor

@ecomodeller

Copy link
Copy Markdown
Member

Hi Henrik, I saw this and reviewed the PR. It seems like it was already completed, tests passed. Did you want me to test on some unseen data or something? Best regards, Michael Allen Bell Marine Data Scientist Marine & Coastal - US [DHI logo] The expert in WATER ENVIRONMENTS DHI Water and Environment Inc. 16-2095 King Kamehameha BLVD Pāhoa, HI 96778 USA Tel: +1 303 937 4488 Direct: +1 720 439 4395 @.@.> www.dhigroup.com<http://www.dhigroup.com/> Privacy policyhttp://www.dhigroup.com/privacy This message contains information which may be confidential and privileged. Unless you are the intended recipient (or authorised to receive this message for the intended recipient), you may not use, copy, disseminate or disclose any information contained in it thereof. If you have received this communication in error, please advise the sender appropriately by reply e-mail, and then delete it. Thank you very much. From: Henrik Andersson @.> Sent: Wednesday, June 3, 2026 10:29 PM To: DHI/mikeio @.> Cc: Michael Allen Bell @.>; Mention @.> Subject: Re: [DHI/mikeio] vertical aggregation (PR #984) [Image removed by sender.]ecomodeller left a comment (DHI/mikeio#984)<#984 (comment)> @miab-dhihttps://github.com/miab-dhi please take a look at this. — Reply to this email directly, view it on GitHub<#984?email_source=notifications&email_token=ANAQ3QSZ2I267QY6F6CZBD346EXMLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSGAZTQMZTGA3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4620383306>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ANAQ3QR3HYIINQI43TABASD46EXMLAVCNFSM6AAAAACZYZOK3KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DMMRQGM4DGMZQGY. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOShttps://github.com/notifications/mobile/ios/ANAQ3QRFAK752CAWVBBOHML46EXMLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSGAZTQMZTGA3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y and Androidhttps://github.com/notifications/mobile/android/ANAQ3QSBZLO7AZUE2W7I5D346EXMLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSGAZTQMZTGA3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI. Download it today! You are receiving this because you were mentioned.Message ID: @.***>

Just wanted to get your opinion if this is useful for you, and make sure that the syntax is optimal, before releasing this into the wild😉.

@otzi5300

otzi5300 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator Author

@tile-timley any comments/suggestions?

@oceandigitalis

Copy link
Copy Markdown

I think this is a very useful contribution and the syntax is clear. Looking forward to start getting rid of my own very time and memory limited solutions.

Does it work to work when sigma-layers crossing border into z-layers (if sigma depth isn't deep enough for ones purposes)?.

Great stuff!

@otzi5300

otzi5300 commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

I think this is a very useful contribution and the syntax is clear. Looking forward to start getting rid of my own very time and memory limited solutions.

Does it work to work when sigma-layers crossing border into z-layers (if sigma depth isn't deep enough for ones purposes)?.

Great stuff!

Yes (and no).

Yes: It works with any combination of sigma and z-layers. It performs a vertical aggregation of the layers in the Dataset/DataArray regardless of whether they are sigma, z, or a mix of both.

No: Currently it only supports aggregation across full layers — you cannot slice at a specific depth (e.g., "give me everything above -5m"). If you need depth-based extraction, you'd first need to select layers manually before applying this operation.

@miab-dhi

miab-dhi commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@otzi5300 I tested this out on a different dataset, 3D sigmaZ dfsu with Phytoplankton results. It worked nicely.

I think this a useful addition, is a huge time saver from the traditional ways of achieving this.

@tile-timley

Copy link
Copy Markdown

Terrific and compared to doing calculation with PFS I think it is fast ("mean" operation included).

We often calculate eg. "vertical mean, 0-8 meter depth". If there was a functionality that allowed to "ds.sel(z-range=(z_min, z_max))" in addition to the existing "ds.sel(layers=(layer_min, layer_max))", that would be amazing.

This is related to the response from @otzi5300 :
"> No: Currently it only supports aggregation across full layers — you cannot slice at a specific depth (e.g., "give me everything above -5m"). If you need depth-based extraction, you'd first need to select layers manually before applying this operation."

@miab-dhi

Copy link
Copy Markdown
Contributor

We often calculate eg. "vertical mean, 0-8 meter depth". If there was a functionality that allowed to "ds.sel(z-range=(z_min, z_max))" in addition to the existing "ds.sel(layers=(layer_min, layer_max))", that would be amazing.

I created a method to extract data from z-val, which extracts a slice at a depth of water either looking from the bottom or down from the surface. We can discuss it more, maybe find a way to implement it in.

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

Successfully merging this pull request may close these issues.

6 participants