search

NOAA-CEFI-Regional-Ocean-Modeling / ocean_BGC

3 stars 4 forks source link

Make geolat and eqn_of_state as required #33

Closed yichengt900 closed 2 months ago

yichengt900 commented 3 months ago

As tiled. This PR is to address issue #29. We have added a fatal error message to handle cases where geolat or eqn_of_state is missing in the call. We have changed geolat and eqn_of_state from optional to required so it will fail at compile time if they are missing on MOM6 side

This PR does not change answers.

andrew-c-ross commented 2 months ago

Would it be better to make eqn_of_state and geolat required instead of optional, so that it fails at compile time instead of run time if they are not provided? Currently there's no way to proceed without these variables anyways.

https://github.com/NOAA-CEFI-Regional-Ocean-Modeling/ocean_BGC/blob/69feb9e27a33422ccbc51bc0331a357740696dc2/generic_tracers/generic_COBALT.F90#L7876-L7877

yichengt900 commented 2 months ago

@andrew-c-ross , good point. Our initial intention was to make generic_tracer.F90 flexible to accommodate COBALTv2 in case we wanted to maintain both versions:

https://github.com/NOAA-CEFI-Regional-Ocean-Modeling/ocean_BGC/blob/69feb9e27a33422ccbc51bc0331a357740696dc2/generic_tracers/generic_tracer.F90#L556-L567

However, since we have now decided to eventually replace COBALTv2, I can remove this part and make both variables as required.

charliestock commented 2 months ago

Hi folks - It would be good to maintain some backward compatibility if it isn't too cumbersome. I will raise this as a discussion issue.