CESR-lab / ucla-roms

GNU General Public License v3.0
5 stars 11 forks source link

unable to provide user control over MARBL options files #25

Open dafyddstephenson opened 5 months ago

dafyddstephenson commented 5 months ago

The MARBL driver currently reads three files:

Currently the filenames are fixed and these files must be located in the directory in which ROMS is executed. I would like to add some flexibility to this.

To this end I have attempted a solution by adding the following lines to my roms.in:

MARBL_biogeochemistry: namelist  tracer_output_list   diagnostic_output_list
  path/to/marbl_in
  path/to/marbl_tracer_list
  path/to/marbl_diag_list

with a corresponding block in read_inp.F:

#ifdef MARBL
        elseif (keyword(1:kwlen) == 'MARBL_biogeochemistry') then
          call cancel_kwd(keyword(1:kwlen), ierr)
          read(input,'(A)',err=95) marbl_namelist_fname
          lstr=lenstr(marbl_namelist_fname)
             mpi_master_only write(*,'(1x,A,15x,A)'),
     &            'MARBL namelist file',marbl_namelist_fname(1:lstr)

          read(input,'(A)',err=95) marbl_tracer_list_fname
          lstr=lenstr(marbl_tracer_list_fname)
             mpi_master_only write(*,'(1x,A,5x,A)'),
     &            'MARBL tracer output list file',marbl_tracer_list_fname(1:lstr)

#ifdef MARBL_DIAGS          
          read(input,'(A)',err=95) marbl_diag_list_fname
          lstr=lenstr(marbl_diag_list_fname)
             mpi_master_only write(*,'(1x,A,1x,A)'),
     &            'MARBL diagnostic output list file',marbl_diag_list_fname(1:lstr)
#endif
#endif

where marbl_namelist_fname, marbl_tracer_list_fname, and marbl_diag_list_fname are declared in and used from marbl_driver.F.

There is an issue here as read_inp is called from inside init_scalars which is called AFTER init_tracers in main.F, at which point MARBL has already been configured and it is too late to read these files.

There is a comment in main.F saying that init_tracers has to be called first, but it is not obvious to me why. Indeed, when I flipped the order, my output was bitwise identical, and the desired functionality was achieved.

Could we use this issue ticket to discuss this implementation or perhaps other strategies for achieving this functionality?

nmolem commented 5 months ago

I've read your comment. Let me look at it to make sure that switching the order of init_scalars and init_tracers is indeed not an issue. I think that you're right, but just want to make sure.

dafyddstephenson commented 5 months ago

Great, thanks for looking into this. There are some more changes to the MARBL driver coming following Jerome's investigations so perhaps this can go in a bulk update PR once we've straightened out the details.

nmolem commented 5 months ago

I looked at it, but as you say, it seems fine to reverse the order of init_scalar and init_tracer. Are thinking that this needs to be added to the current 'stable' version, or can this be part of development. It doesn't seem to be a bug fix per se. Reason I'm bringing this up is that there is stale code in at least init_scalars that I would like to remove, but I don't want to mess with 'stable' unless its really needed.

dafyddstephenson commented 5 months ago

Thanks for looking into that. Although these aren't bugfixes, they do add quite fundamental functionality, so it would be ideal to add this in with the MARBL bugfixes that Jerome is currently testing. Perhaps the clean-up in init_scalars can happen separately, either on its own branch, or down the line? I am just waiting on input files for bgc_real to finalise the test routines, after which a tagged stable release should take too long.