ESCOMP / ESCOMP/MOSART

Fix logic for ctl%rof_from_glc

Open
#114 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Fortran
Stars
12
Forks
35
PR merge metrics
No merged PRs in 30d

Description

This is a minor issue that doesn't cause any problems in practice right now, but should probably be fixed to avoid creating problems in the future.

The logic for `ctl%rof_from_glc` is:

https://github.com/ESCOMP/MOSART/blob/bd5c2707f6957e32f6d3d3ad2efa2c3e5261b044/src/cpl/nuopc/rof_import_export.F90#L231-L235

Note the duplication of `Fgrg_rofl` as opposed to `Fgrg_rofi`.

I think the right logic here is something like:

```f90
if (fldchk(importState, 'Fgrg_rofl') .and. fldchk(importState, 'Fgrg_rofi')) then
ctl%rof_from_glc = .true.
else if (fldchk(importState, 'Fgrg_rofl') .or. fldchk(importState, 'Fgrg_rofi')) then
! ABORT WITH AN ERROR: I think it should never be the case that one of these fields is present but not the other; I think if it could be the case, some other code may need to be generalized to allow for this possibility
else
ctl%rof_from_glc = .false.
end if
```

But I'd like a second set of eyes on that to make sure it looks right.

Tagging @olyson who rediscovered this issue that I had mentioned as a side note in https://github.com/ESCOMP/MOSART/issues/103#issuecomment-2477742884 but never followed up on. And tagging @ekluzek or @slevis-lmwg , who were involved in that earlier discussion.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.