Skip to content

Fix bug with alarms after clock direction change.#29

Open
TerribleNews wants to merge 1 commit intoesmf-org:developfrom
TerribleNews:develop
Open

Fix bug with alarms after clock direction change.#29
TerribleNews wants to merge 1 commit intoesmf-org:developfrom
TerribleNews:develop

Conversation

@TerribleNews
Copy link

I have been working on adjoint development for GCHP and found a problem with the history alarms after running the clock forward and then reversing it. It appears to be a problem with the checkRingTime command which is called in a for loop but changes a variable in the clock, userChangedDirection, which is then read in subsequent loop iterations but the original value has been overwritten. This small change saves the value and resets it before each call to checkRingTime so that the correct branch is taken in there, and then the last one sets clock->userChangedDirection to false, which should be the value after the loop has completed. This is probably not the most efficient fix, but it's the one that requires the smallest code change.

@sdeastham
Copy link

Can anyone from the ESMF team confirm if this bug is still present? It is unfortunately a major issue for an adjoint application being developed for the NASA GEOS ecosystem.

@danrosen25
Copy link
Member

@sdeastham @TerribleNews
The ESMF Team is reworking the Time Manager and Alarm classes.
https://github.com/esmf-org/esmf/tree/newalarm
We're getting ready to test the above branch, which may fix the alarm bug discussed here. I'll make a note of this issue on the branch PR.

@danrosen25 danrosen25 mentioned this pull request Apr 26, 2023
@sdeastham
Copy link

Thanks @danrosen25 ! I'll keep an eye on the new class, that's good to hear. @TerribleNews - any idea if the new alarm class would inherently fix this issue?

@billsacks billsacks mentioned this pull request Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants