confusing behavior if register tracer functions are called with associated CS

[klindsay28]

In most, perhaps all, passive tracer modules in src/tracer, the register tracer function starts with a block like

  if (associated(CS)) then
    call MOM_error(WARNING, "DOME_register_tracer called with an "// &
                            "associated control structure.")
    return
  endif
  allocate(CS)

Except for register_MOM_generic_tracer, the return value of the function is not set before the return statement inside this conditional. So it seems like an uninitialized value is getting returned. That seems problematic to me.

In register_MOM_generic_tracer, the return value is set to .false. prior to this block. So if you call register_MOM_generic_tracer with an associated CS, say by calling register_MOM_generic_tracer twice, you return .false.. In the call to register_MOM_generic_tracer in MOM_tracer_flow_control.F90, that would turn the tracer module off. That seems surprising to me.

Is there a use case where a developer intentionally calls a register tracer function with an associated CS? I'm not seeing one, but perhaps my imagination isn't big enough.

If not, would it make sense to call MOM_error(FATAL, ... if this happens? The return statement can then be dropped, and you don't need to be concerned with what value you're returning.

[Hallberg-NOAA]

You make very good points, @klindsay28.

I do not recall why we decided to issue WARNING messages instead of FATAL errors when the various tracer register routines are called multiple times, and I can not image an appropriate use case for multiple calls with the same control structure either. (I can envision cases where these routines are called multiple times by different ensemble members, but then each ensemble member should use its own control structure.)

I think that I agree with your suggestion, that the right answer is to make all of the error messages for associated control structures being passed into to the various register_tracer routines into FATAL errors.

[marshallward]

Just a heads up that if we do decide to rebrand these as variables rather than pointers, then checks like these would either be removed or replaced with control flags.

I agree with Keith's observation, and I encountered similar cases while working through other memory errors.

[Hallberg-NOAA]

The code addressing this issue has now been merged onto the main branch of MOM6 as a part of #1582, with the specific changes coming in from NOAA-GFDL#182.