Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#737: SCHISM Coastal Formulation #810

Draft
wants to merge 55 commits into
base: master
Choose a base branch
from

Conversation

PhilMiller
Copy link
Contributor

@PhilMiller PhilMiller commented May 3, 2024

Resolves #737

Additions

  • CoastalFormulation abstract base class
  • Support classes for relevant DataProvider instantiations that CoastalFormulation will need
  • SCHISM-based implementation of CoastalFormulation

Changes

  • parallel_utils.h gains the MPI size/rank global variables, and has its contents split from the header to a source file
  • Bmi_Fortran_Adapter gains a constructor that implements the prototype MPI communicator setup protocol

Testing

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist (automated report can be put here)

Target Environment support

  • Linux
  • macOS

@PhilMiller PhilMiller force-pushed the PhilMiller/coastal-schism branch 5 times, most recently from 24cb892 to 93b3bf7 Compare June 26, 2024 19:36
@PhilMiller PhilMiller force-pushed the PhilMiller/coastal-schism branch from 93b3bf7 to 2050061 Compare August 8, 2024 00:05
@PhilMiller PhilMiller force-pushed the PhilMiller/coastal-schism branch from d38a9d9 to 4d55b6c Compare August 16, 2024 20:25
@PhilMiller
Copy link
Contributor Author

This is totally not ready for review, but it's now a demonstration of code that drives SCHISM through the NGen Fortran BMI adapter, initializing, setting variables, updating, and finalizing.

I think for our deliverable, we'll be able to use NetCDF files and the associated provider rather than the forcing engine, but I'll have to see. Justin, in the meanwhile, this should give a decent sense of what I'd like for an unstructured mesh version of the ForcingsEngineDataProvider. Don't look at anything I've written on that front, especially, as set in stone. It's all meant to be just enough to fill out all aspects of the design.

Note that there's still a shortfall in the SCHISM BMI code that limits this to running on just 1 MPI rank for now, since in partitioned form, some arrays are split while others aren't.

@PhilMiller PhilMiller linked an issue Aug 27, 2024 that may be closed by this pull request
@PhilMiller PhilMiller force-pushed the PhilMiller/coastal-schism branch from c7392f8 to eaac987 Compare September 17, 2024 19:48
@PhilMiller PhilMiller changed the title #737: Initial draft of SCHISM Coastal Formulation #737: SCHISM Coastal Formulation Oct 1, 2024
@PhilMiller
Copy link
Contributor Author

PhilMiller commented Dec 2, 2024

I've found that I need to disable Fortran array bounds checking in the BMI implementation to work with this on GNU Fortran, because the array arguments that come in have. reported bounds of 0.

This maybe arose new in gfortran-14. I haven't gone back to older versions to check.

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.

SCHISM Coastal Formulation Unit-ish test Implement SCHISM 'coastal realization' formulation
1 participant