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

Update SIS2 to use the new IO interface #196

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

marshallward
Copy link
Member

The legacy file_type and fieldtype handles for field IO are replaced with the newer MOM IO types (MOM_infra_file, MOM_field), which provide new FMS-agnostic interfaces to the framework IO.

This also conforms to the new requirement of FMS IO requiring a domain. (Note that this may be relaxed in the future.)

Thanks to Kate Hedstrom (@kshedstrom) for identifying the issue and providing the first draft to this fix.

The legacy file_type and fieldtype handles for field IO are replaced
with the newer MOM IO types (MOM_infra_file, MOM_field), which provide
new FMS-agnostic interfaces to the framework IO.

This also conforms to the new requirement of FMS IO requiring a domain.
(Note that this may be relaxed in the future.)

Thanks to Kate Hedstrom (@kshedstrom) for identifying the issue and
providing the first draft to this fix.
@marshallward
Copy link
Member Author

@kshedstrom I seem unable to add you as a reviewer, but if you have a chance it would be good to get you feedback.

@marshallward
Copy link
Member Author

There is still an error here: ice_model.res.nc is still not domain-decomposed even though PARALLEL_RESTARTFILES is True in SIS_parameter_doc.all.

This is the same error as in #195, and which I had thought this patch would address, but I suppose not. I will continue to look into it.

@marshallward
Copy link
Member Author

marshallward commented Jun 9, 2023

Apparently if the non-domain-decomposed file is seen, it ignores PARALLEL_RESTARTFILES and does a single file anyway. Then, when the restart rolls around, it freaks out if the file is not domain-decomposed.

Either way this is consistent, but I find it weird that it uses the form of the existing file in place of the user-specified flag.

And FWIW, this is nothing that #196 would have fixed; #195 would have been fine here.

@kshedstrom
Copy link
Contributor

Yes, it's been working for me. Sorry, I thought I had posted, but I guess not.

@marshallward
Copy link
Member Author

Sorry @kshedstrom, those comments weren't directed to you, I should have been more clear. We are still seeing errors with FMS1, so we can't merge this yet.

@marshallward
Copy link
Member Author

I think the associated issues on MOM6 have been fixed, and this is now ready for review.

@marshallward
Copy link
Member Author

@kshedstrom I've tested this in FMS1 with our CI and tried to test it with FMS2 to the best of my ability, but if you have a chance to test it in one of your FMS2 runs that would be a big help.

@kshedstrom
Copy link
Contributor

Yes, it's been working fine for me.

@marshallward
Copy link
Member Author

@Hallberg-NOAA @adcroft This is ready for review

@adcroft
Copy link
Member

adcroft commented Jun 20, 2023

@adcroft adcroft merged commit 88d9e32 into NOAA-GFDL:dev/gfdl Jun 20, 2023
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