Delft3DFM Application and Spatial Functionality

[caitlinturner]

I have added the following components to dorado:

1. A walk-through notebook for how to use implement dorado with Delft3D-Flexible Mesh models (unstructured_grid_Delft3dFM.ipynb) which mirrors the unstructured walk-through with Anuga. This Delft3D use case is an unsteady example with particles initially located within the region of interest (ROI). Notebooks - dorado use for Delft3DFM #55
2. A model component that allows users to calculate exposure time spatially (dorado.spatial.py). This function further rasterizes the model into "chunks" and determines what the exposure time of particles are at particular thresholds of the cumulative distribution function (CDF) (i.e, $E_{50}$, $E_{75}$, and $E_{90}$, when 50, 75, and 90 percent of particles leave the region of interest, respectively) for each chunk. This is used in two ways: Adding Model Component - spatial exposure time #54

1. System-wide: quantifies the amount of time a particle starting in a chunk spends in the larger region of interest. This is used to determine how long particles originating in one location take to leave a basin.
2. Localized: quantifies the amount of time particles starting in a chunk spend inside that chunk. This is used to capture localized flows and identify "hot spots" of increased exposure times due to circular motion.

3. A walk-through notebook for how to use the spatial model component (spatial_exposure_time_example_Delft3DFM.ipynb). This notebook walks through the steps to create the spatial model, what thresholds mean, and provides figures. Notebooks - dorado use for Delft3DFM #55
4. A unit test for dorado.spatial.py (test_spatial.py) to ensure installation is correct.
5. An additional plotting function was added to dorado.routines.py (dorado.routines.plot_exposure_time_thresholds) to create CDF plot with thresholds labeled and printed out to the user. routines.py exposure_plot update #56

A second set of eyes (Mark Piper from CSDMS) checked over code, functionality, and unit testing.

[codecov]

Codecov Report

❌ Patch coverage is 97.26027% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.57%. Comparing base (85c3cc3) to head (5ce23ea).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
dorado/spatial.py	97.26%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
- Coverage   97.65%   97.57%   -0.09%     
==========================================
  Files           2        3       +1     
  Lines         555      701     +146     
==========================================
+ Hits          542      684     +142     
- Misses         13       17       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[elbeejay]

Awesome @caitlinturner. @wrightky is this something you want to review/test out or should I?

[wrightky]

Awesome @caitlinturner. @wrightky is this something you want to review/test out or should I?

These additions look very cool! I might have time for a full review later this the week, but if you have time @elbeejay feel free.
My first quick thought is make sure to update __init__.py to import the new spatial.py module and increment the version number -- presumably a minor upgrade to 2.6.0

[caitlinturner]

Thank you! I noticed your notebook examples were in their "run" form. Would y'all like me to do this? When learning the code, I used that version quite a bit instead of running it.

[elbeejay]

I can probably look things over tomorrow evening. I don't have a preference on notebook form

[Copilot]

Pull Request Overview

This PR adds comprehensive Delft3D-Flexible Mesh support and spatial functionality to dorado, focusing on exposure time analysis capabilities. The implementation provides both system-wide and localized exposure time calculations with spatial visualization.

• Adds complete Delft3D-FM unstructured grid support with data conversion utilities
• Implements spatial exposure time analysis with chunking and threshold computation
• Provides comprehensive documentation through example notebooks and unit tests

Reviewed Changes

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

Show a summary per file

File	Description
tests/test_spatial.py	Unit tests for spatial exposure time functionality with test fixtures and plotting validation
examples/unstructured_grid_Delft3dFM.ipynb	Complete walkthrough notebook for Delft3D-FM integration with Lake Pontchartrain example
examples/spatial_exposure_time_example_Delft3DFM.ipynb	Tutorial notebook demonstrating spatial exposure time calculations and visualization
dorado/spatial.py	Core spatial analysis module implementing system-wide and localized exposure time computations
dorado/routines.py	Enhanced plotting functions with threshold visualization capabilities

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[elbeejay]

My comments are all minor, the only change that I think needs to be made prior to merging is the fix to the end_time variable name in routines.py. I think I'd be okay with opening up issues to look into some of the other items like streamlining the plotting functions etc.

[elbeejay]

Apologies for testing out this GitHub AI code review feature but I saw the button and couldn't help it @caitlinturner - although the comments the bot made also seem very minor!

[elbeejay]

Awesome @caitlinturner. @wrightky is this something you want to review/test out or should I?

These additions look very cool! I might have time for a full review later this the week, but if you have time @elbeejay feel free. My first quick thought is make sure to update __init__.py to import the new spatial.py module and increment the version number -- presumably a minor upgrade to 2.6.0

I agree with these suggestions for __init__.py. I'd also suggest going ahead and adding yourself to the copyright / authorship lines in the documentation conf.py

[caitlinturner]

Apologies for testing out this GitHub AI code review feature but I saw the button and couldn't help it @caitlinturner - although the comments the bot made also seem very minor!

Totally fine! Its interesting to see what it picked up!

[caitlinturner]

I have made the above updates and suggestions! I did change the notebooks to be in a run form. While learning how to use the code when I first started this work, it was helpful to refer to the 'run' version, although I did remove long outputs that made things look clunky.

Thank you for the feedback, @elbeejay!

[wrightky]

The code looks good to me! Y'all have already addressed most of the same things I would've commented on. Here are just a few extra suggestions:

• In the example notebook, the third citation for the hydroshare repo for your paper has a broken hyperlink. I think you just need to update the doi part from doi: to doi.org. Also, congrats on the paper! Excited to read it.
• I see that you updated __init__.py, but don't forget to also increase the version number to 2.6.0 that way we can trigger a new release for the package.
• As (kind of) pointed out by @elbeejay's AI, the docstrings for spatial.systemwide and spatial.localized are pretty similar, and I do think it might be a bit difficult for users to tell at a glance how they differ from each other. I think it would help to add more detail to the description to emphasize how they differ.
• Currently, the function compute_thresholds defines two nested functions (exposure_time_stats and median_absolute_deviation_filter) and one of those defines a third nested function (pval). These nested functions are generally best to avoid if possible. I think (?) you can directly un-nest median_absolute_deviation_filter without any changes, and I think exposure_time_stats can likewise be un-nested if you just pass along a few extra inputs. pval is pretty minor and can probably just stay as is. I recommend trying to see if you can tease apart those functions from each other.

[caitlinturner]

Okay, I think I have addressed everything!
All tests still pass and results come out the same in the notebook. Thank you for the feedback!

[caitlinturner]

Thank you both again for checking this over. If there are any other steps I need to take, please let me know!
This is a really great tool to work with and I am excited to keep working with it!

[elbeejay]

@wrightky merge when you're ready