Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix: fixes #140 #143

Merged
merged 4 commits into from
May 2, 2022
Merged

Conversation

bena-nasa
Copy link
Contributor

@bena-nasa bena-nasa commented Apr 28, 2022

Fixes #140
Also needs this PR from chem as well to fix the issue:
GEOS-ESM/GEOSchem_GridComp#182

@bena-nasa bena-nasa requested a review from a team as a code owner April 28, 2022 17:09
@bena-nasa bena-nasa added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. bug Something isn't working labels Apr 28, 2022
@mathomp4 mathomp4 changed the title fixes #140 Hotfix: fixes #140 Apr 28, 2022
Copy link

@adarmenov adarmenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bena-nasa the way this PR and the accompanying ACHEM PR were formulated, e.g. 'Variables being provided by ExtData must be MAPL_RestartSkip' makes me wonder if the aforementioned requirement breaks the symmetry between variables provided by ExtData or 'model' components. Could you please clarify, if we need to differentiate between ExtData imports and non-ExtData imports?

@adarmenov I'm not sure what you are asking, these changes I've made here and in ACHEM are inline with how we have handled all the other variables that have been provided via ExtData every since we started using ExtData.

There's no way for the component to know if an IMPORT it request will come from ExtData or be satisfied by a connectivity higher up in the MAPL hierarchy, that's the point of the componentization. It's up to the user to make sure they have the right Restart option to the MAPL_AddImportSpec call. We were handling this right in GOCART1g, GEOSChem, TR, GMI etc... This just got overlooked when GOCART2G was created and since I guess no one really ran achem with replay before Achem got turned on by default it also got missed there.

Copy link
Collaborator

@amdasilva amdasilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks resonable and appropriate as it implements the same behavior in Legacy GOCART

@amdasilva amdasilva merged commit 02262ad into main May 2, 2022
@amdasilva amdasilva deleted the hotfix/bmauer/#140-restartskip_statespec branch May 2, 2022 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0-increment Replays are not 0-diff
4 participants