Add MARBL driver

[dafyddstephenson]

Hi all, here's the long-awaited PR with the marbl driver. I've merged in all the latest changes from main and run all the test cases. I have a separate toy problem for MARBL located here and this is also running as expected.

MARBL will need to be obtained and compiled for the driver to work. It can be found here. ROMS uses the environment variable $MARBL_ROOT, which should point to wherever that repo is cloned. The driver was built around tag 0.45. Documentation for MARBL can be found here.

As we've discussed in our meetings, there are maybe some coding choices I've made that would work better with ROMS if implemented differently - let's hash out the discussion here.

Still missing from ROMS-MARBL currently are river and seafloor inputs of BGC tracers (MARBL expects iron inputs from HTVs and sediment, while river inputs are typically incorporated in the surface forcing). Once we've got this merged we can perhaps work on incorporating these features.

Thanks! Dafydd

[dafyddstephenson]

Thanks for the meeting @nmolem , before the merge I'll attempt to :

• tidy up the character arrays where they haven't been
• move the wind-related changes into surf_flx.F
• generally reduce the footprint of MARBL (e.g. ecosys2_init call, moving the saved state related things to the MARBL side excepting an SR call in get_init and basic_output)
• Play with the call marbldrv_column_physics which is potentially passing arrays unnecessarily so as to avoid indexing issues

[nmolem]

Thanks @dafyddstephenson! Your summary of our conversation looks good. Adding the observation that the pull request changes to the expanse scripts just change the things that are specific to us, to things that are specific to you. Maybe we could work with an environment variable for those.

[dafyddstephenson]

Here's a summary of changes so far:

• Went back through all BGC related CPP checks and eliminated a large number of superfluous ones while introducing a couple of additional ones to skip over redundant blocks of BEC code when using MARBL. In doing this I patched a pretty significant potential bug whereby I believe SRs reading BGC forcing were not being called when using MARBL. The only remaining check I'm unsure about is in comm_vars.F .
• Fixed any remaining character arrays à la compliant branch.
• moved code blocks concerning the reading of MARBL saved state variables from get_init to a subroutine in marbl_driver which is called from get_init
• moved code blocks concerning the creation and writing of MARBL saved state variables in the restart file from basic_output to subroutines in marbl_driver which are called from basic_output
• changed call to marbldrv_column_physics in step3d_t_ISO to pass fewer arguments and correctly pass arrays with nonstandard bounds defined on GLOBAL_2D_ARRAY more cleanly
• Changed use of landmask in marbl_driver to rmask - landmask didn't appear to be working correctly and required another argument in the SR call.
• restructured do_roms_expanse.sh scripts in Example cases so account key can be a bash variable
• moved declaration and allocation of uwnd and vwnd into surf_flux module, avoiding a circular dependency between tracers -> marbl_driver -> bulk_frc -> tracers without creating a new module

[nmolem]

Hi Daffyd, Pierre and I are going through the code. In step3d_t_iso.F, in the call to marbldrv_column_physicsm, you're passing bec2_diag_2d andbec2_diag_3d. Could you change that to having the driver just use the module to get access to those vars (like bec does)

[dafyddstephenson]

Hi @nmolem , I went through and pared down the call as much as possible but unfortunately that one had to stay in: marbl_driver is used by bgc_ecosys_vars which is where those arrays are declared. They would have to be moved to avoid passing them in with the call.

I should add that I've been working on some pretty substantial changes today after Ulla highlighted that I haven't provided the user with control over which tracers/diagnostics are written to the output file, so please don't merge this in quite yet.

[nmolem]

Ok, it might be worth it to see if those vars can be defined in a separate module that would eliminate the circular reference (similar to the move of uwnd,vwnd. Additionally, I pushed another commit to main, prompted by the realization that I had introduced a big error on march 14 to bulk_frc. Also, we changed wrt_t_dia and nt_passive. The last one because we (you) had both nt_passive in param.opt and nt_pass in tracers. Hopefully it will be easy to resolve the conflicts that resulted.

[dafyddstephenson]

I think to do this I would need to set up an entirely separate procedure for MARBL diagnostics vs BEC diagnostics (allocating and acting on a different set of diagnostics arrays). I imagine this would largely be copy and pasting code from BEC into the MARBL driver and calling it if the MARBL cpp key is defined. I'm already doing quite a bit of work on diagnostics right now so I might as well take a look and let you know how involved this would be tomorrow. The conflicts should also be easy enough to resolve based on what you've said.

[dafyddstephenson]

For now I've addressed the need to pass the diagnostics arrays to MARBL in the subroutine call by defining pointers to the diagnostics arrays in the MARBL driver. I've also merged in the latest changes from main and run the example cases on OSX and Expanse.

I still would like to make another commit or two implementing user control over output variables before the merge, which I'll do tomorrow.

[dafyddstephenson]

OK, I've added that additional feature of user output control (and following Scott's issue on the call this morning remembered to go back and ensure single-cpu text file reading followed by broadcasting, which had been on my to do list for a while). I think this is about ready to be closed (🤞) but am happy to do another pass after you've both taken a look.

[nmolem]

Sorry Dafydd, I was working on another project for a while. I'll look at it today.