GEOS-ESM / GEOSgcm_GridComp

Repository containing the physics and IAU code for the GEOS Earth System Model
Apache License 2.0
9 stars 7 forks source link

Remove `IS_FCST` and clean up associated logic in replay `TYPE` #911

Closed sanAkel closed 4 days ago

sanAkel commented 7 months ago

Testing by @rtodling has found that these lines of code: https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/8faac4966314203266371f2ae5bea3b77eb88302/GEOSagcm_GridComp/GEOS_AgcmGridComp.F90#L1730-L1734

are unnecessary, cause confusion and (potential) bugs. @atrayano is also aware of these lines of code.

@rtodling please add your notes as needed.

This IS_FCST must be removed also the above mentioned if block cleaned up.

IF
...
ENDIF
tclune commented 7 months ago

@sanAkel This is "assigned" to me. I'm assuming that this is just meant to keep me informed rather than actually having me implement the fix?

sanAkel commented 7 months ago

@tclune, perhaps you would be able to (re-) assign someone to it?

mathomp4 commented 7 months ago

I mean, I could do the coding if I knew what is the "right" way to fix it. Could you and/or @rtodling let me know?

rtodling commented 7 months ago

Guys, before we change anything I suggest we do a little investigation on who is (if anyone) using this flag set to 1. If nobody is using the flag the simplest thing to do is to wipe out the implementation associated w/ the option 1.

I also strongly suggest talking to Larry Takacs, He might recall what the motivation might have been for distinguishing between a forecast and a "freerun" ... I sincerely cannot see the difference from an intellectual point of view, but clearly in the code lots of things happen that differ the two cases ... so not sure ... I suggest being careful w/ this.

I'll also add that there are other fish to fry w/ higher priority so I would leave this here as a note for us to remember, but would not spend too much time on this.

atrayano commented 7 months ago

Ricardo, I agree that talking Larry Takacs is the right thing to do. I am more or less aware of the initial motivation: this has to do with the so-called bias correction done in Agcm. But again, the whole bias correction was Larry's idea, and the implementation is done also by him.

sanAkel commented 7 months ago

Ricardo, I agree that talking Larry Takacs is the right thing to do. I am more or less aware of the initial motivation: this has to do with the so-called bias correction done in Agcm. But again, the whole bias correction was Larry's idea, and the implementation is done also by him.

Thanks @mathomp4, @rtodling, @atrayano

Sometime - not "soon", I plan to have a chat with @lltakacs Reg replay. Will add this to my list of questions.