marbl-ecosys / MARBL

Marine Biogeochemistry Library
https://marbl-ecosys.github.io
Other
14 stars 25 forks source link

Is "abort" the best description for what MARBL does when an error occurs? #384

Open mnlevy1981 opened 3 years ago

mnlevy1981 commented 3 years ago

@klindsay28 and I were discussing this in the context of the description of two MARBL settings (one that I added in #381, and one that I copied the wording from when adding the new setting) and several variables that are dependent on one of the settings:

$ grep -i "will abort" src/marbl_settings_mod.F90
       bftt_dz_sum_thres,          & ! MARBL will abort if abs(1 - sum(bot_flux_to_tend)) exceeds this threshold
       Jint_Ctot_thres_molpm2pyr,  & ! MARBL will abort if abs(Jint_Ctot) exceeds this threshold
       Jint_Ctot_thres,            & ! MARBL will abort if abs(Jint_Ctot) exceeds this threshold (derived from Jint_Ctot_thres_molpm2pyr)
       Jint_Ntot_thres,            & ! MARBL will abort if abs(Jint_Ntot) exceeds this threshold (derived from Jint_Ctot_thres)
       Jint_Ptot_thres,            & ! MARBL will abort if abs(Jint_Ptot) exceeds this threshold (derived from Jint_Ctot_thres)
       Jint_Sitot_thres,           & ! MARBL will abort if abs(Jint_Sitot) exceeds this threshold (derived from Jint_Ctot_thres)
       Jint_Fetot_thres,           & ! MARBL will abort if abs(Jint_Fetot) exceeds this threshold (derived from Jint_Ctot_thres)
       CISO_Jint_13Ctot_thres,     & ! MARBL will abort if abs(CISO_Jint_13Ctot) exceeds this threshold (derived from Jint_Ctot_thres)
       CISO_Jint_14Ctot_thres,     & ! MARBL will abort if abs(CISO_Jint_14Ctot) exceeds this threshold (derived from Jint_Ctot_thres)
    lname     = 'MARBL will abort if abs(1 - sum(bot_flux_to_tend)) exceeds this threshold'
    lname     = 'MARBL will abort if abs(Jint_Ctot) exceeds this threshold'

MARBL is aborting in the sense that it is returning to the driver without any further computation, but the library doesn't have the ability to cause whatever program is calling it to abort... so the wording is a little misleading. An open question is "how should we describe what these variables do?"

If the answer is to drop the verb "abort" from the comment, we may also want to rethink the name of the labort_MARBL flag that we use to report an error to the driver.

It may make sense to tackle this issue alongside #362, which will also touch on the behavior of labort_MARBL. For example, deciding to switch from labort_MARBL to an error code to clarify what MARBL is doing would be a good time to have all the interface subroutines reset the error code to 0 when called (or to take it off the MARBL interface altogether), which would address the earlier issue.