Skip to content

timestep limiter diagnostics (csv) + unified timestep analysis #284

Closed
EgemenAnder wants to merge 2 commits intoDeltares:mainfrom
EgemenAnder:5timestep-limiter-diagnostics-(CSV)-+-unified-timestep-analysis
Closed

timestep limiter diagnostics (csv) + unified timestep analysis #284
EgemenAnder wants to merge 2 commits intoDeltares:mainfrom
EgemenAnder:5timestep-limiter-diagnostics-(CSV)-+-unified-timestep-analysis

Conversation

@EgemenAnder
Copy link
Copy Markdown
Contributor

I combined the timestep analysis idea from #267/#280 with my timestep diagnostics implementation.

Main changes:

  • The limiting determination is now done outside the parallel momentum loop (after compute_fluxes) in sfincs_timestep_diagnostics.f90
  • timestep_analysis produces the NetCDF map variables average_timestep and times_limiting (with coordinates = "x y").
  • Diagnostics outputs include x/y for the limiting UV point, and the dtmax-cap case is explicitly reported as reason=2 (ip/nm/nmu/x/y set to 0).

I rebuilt and ran short CPU tests (diag off/on, every/interval writes, analysis on, and dtmax-cap). All runs completed cleanly and outputs look consistent.

Inputs/Outputs

  • Inputs (sfincs.inp):
    • timestep_diagnostics (0/1)
    • dt_timestep_diagnostics (s; 0 = every timestep)
    • timestep_analysis (0/1)
  • Outputs when timestep_diagnostics=1:
    • timestep_diagnostics.csv
    • timestep_diagnostics_domain.csv
  • Outputs when timestep_analysis=1 (netcdf map):
    • average_timestep
    • times_limiting

Edge cases handled

  • When dtmax is limiting, diagnostics report reason=2 with limiter metadata set to zero.

Best,
Egemen

@keesnederhoff keesnederhoff self-requested a review March 11, 2026 19:21
@keesnederhoff keesnederhoff self-assigned this Mar 11, 2026
Leynse added a commit that referenced this pull request Mar 13, 2026
Leynse pushed a commit that referenced this pull request Mar 19, 2026
* Initial commit of diagnostics

* Moved diagnostic to separate module

* - Also add sfincs_timestep_diag.f90 to Makefile.am for Linux compilation

* - Don't want this change back to older VS version

* Refactored code after discussion with Maarten:
- determination required timestep should be based over the flux uv point instead of cell centre nm
- renaming variables
- doing calculation of average required timestep per cell in finalize
- naming to _analysis consistently
- make a initialize_timestep_analysis subroutine
- keep compute_cell_min_timestep simple in sfincs_momentum instead
- fix openacc data management

* - Working netcdf output for 'average_required_timestep' and 'percentage_limiting_timestep'
- Note that percentage limiting timestep might not be visible when plotting as continuous/patches, if simulation is always limitied by 1 flux link (2 cells)

* - Add to docs, as done by PR #284

* - Add logstring message of most limiting cell, x&y-coordinates and % of computation

* - Fix for Docker CPU compilation

* Made it work properly for quadtree (not tested for regular). Some refactoring.

* - Fix situation where a neumann msk=6 cell does not find any neighbouring msk=1 cell, then set to 0
- Windows compiler already stepped over this issue, but gfortran does not

* Updated documentation to match source code

* - Fix merge issue

* - bump version

---------

Co-authored-by: Kees Nederhoff <kees.nederhoff@deltares.nl>
Co-authored-by: Leynse <Tim.Leijnse@deltares.nl>
Co-authored-by: Tim Leijnse <timleynse94@gmail.com>
Co-authored-by: Maarten van Ormondt <maarten.vanormondt@deltares.nl>
Co-authored-by: Maarten van Ormondt <maarten.vanormondt@deltares.nl>

With thanks to EgemenAnder for providing feedback on implementating this functionality!
@Leynse
Copy link
Copy Markdown
Collaborator

Leynse commented Mar 19, 2026

Merged in PR #280 , implementation was done a bit differently with output on netcdf grid, but combined functionality deemed relevant, including outputing the uv, x&y locations

@Leynse Leynse closed this Mar 19, 2026
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.

3 participants