Update src/drivers/schism/fabm_driver.h with MASK definitions

[platipodium]

Recent versions of SCHISM need the MASK_TYPE and MASK_VALUE to be defined in the driver. This Commit adds the relevant definitions. We also update copyright and license (placed in public domain).

[bolding]

Hi Carsten

Will it break older versions of SCHISM?

Karsten

[platipodium]

Will it break older versions of SCHISM?

No, it will not break older versions of SCHISM. As there is no mask in those, the defines will simply be not used at all.

[jornbr]

Just to note that if you do provide _FABM_MASK_TYPE_, FABM will demand that the mask has been provided when you call model%start. Thus, in theory, this PR could break backward compatibility. But the question is whether there is a SCHISM-FABM coupler out there that is (a) mask-free and (b) compatible with the latest FABM releases. If not, there is no harm updating SCHISM's fabm_driver.h in the current master.

A separate question that has no implication for this PR, but maybe for future development: does the mask really need to be depth-explicit? If SCHISM either uses all vertical layers (wet point), or none at all (dry point), you could tell FABM that the mask is horizontal-only by defining _FABM_HORIZONTAL_MASK_. That can offer noticeable performance gains.

[platipodium]

in theory, this PR could break backward compatibility. But the question is whether there is a SCHISM-FABM coupler out there that is (a) mask-free and (b) compatible with the latest FABM releases.

Thanks for the hint to this implicit code breaking potential. There is as far as I know only one fabm_schism coupler under the control of the schism-dev community, and I lead that development. There are uses of this coupler in very old versions with FABM0, not setting a mask. Set_mask was implemented in the coupler in May 2016, for both FABM0 and FABM1. We should be willing to risk breaking code that is > 8 years old and FABM0.

[platipodium]

If SCHISM either uses all vertical layers (wet point), or none at all (dry point), you could tell FABM that the mask is horizontal-only by defining FABM_HORIZONTAL_MASK. That can offer noticeable performance gains.

Thanks for this hint, I checked the code, there are no uses of a non-horizontal mask, so we should indeed define _FABM_HORIZONTAL_MASK_

[bolding]

right now there are errors :-)

https://github.com/fabm-model/fabm/actions/runs/8737222365/job/23974279396?pr=64

[platipodium]

right now there are errors :-)
https://github.com/fabm-model/fabm/actions/runs/8737222365/job/23974279396?pr=64

I am looking into these, and can reproduce them locally. schism host is very similar to fvcom host. The only difference is that in schism the _FABM_DEPTH_DIMENSION_INDEX_ 1 whereas in fvcom _FABM_DEPTH_DIMENSION_INDEX_ 2. In fact, changing the depth dimension index to 2 in the schism host for test_host passes the test harness. I can reproducibly produce the test harness failure by changing this value only. Is this potentially a problem in the test harness itself and not in the driver?

[jornbr]

Exactly right - it's a problem in the test harness (SCHISM uses a quite unique combination of settings in fabm_driver.h, so it shows up only there). Looking into it...

[jornbr]

The test harness has now been updated. Can you pull the latest changes from FABM master into your PR branch? Then the tests should pass, and we'll merge it in.

[platipodium]

The test harness fails with two models:

2 ERRORS! Check the logs:
test_harness_schism_fabm-nersc-ecosmo.log
test_harness_schism_fabm-npzd-carbonate.log

The error is similar:

Simulating with 542 wet cells...
 S,T,P   30.000000000000000        15.000000000000000        1.0000000000000000     
 CONCS  -196078431372549.03        2.0573000000000002E-003   6.0134700169990685E-154   100.00000000000000        2.3164141992681716E-155   0.0000000000000000       -196078431372549.03        1.4917138461599308E-154   3.5382693357715792E-004
co2_dyn:POLYCO: Haltafall iteration did not converge.
Note: The following floating-point exceptions are signalling: IEEE_DIVIDE_BY_ZERO IEEE_UNDERFLOW_FLAG IEEE_DENORMAL

[jornbr]

These runtime failures likely relate to the test harness requesting source terms in masked cells. I have updated the test harness code in master to avoid this, and also added more logic to skip calculations in masked cells/below bottom for models like SCHISM (i.e., with the vertical dimension vectorized, variable bottom index). That's in preparation for use with _FABM_HORIZONTAL_MASK_. Can you try merging in the latest master? We'll the rerun the tests.