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

Feature/increment write #15

Merged
merged 9 commits into from
Sep 13, 2023
Merged

Feature/increment write #15

merged 9 commits into from
Sep 13, 2023

Conversation

phlndrwd
Copy link
Collaborator

@phlndrwd phlndrwd commented Sep 12, 2023

This PR introduces the necessary changes to MONIO for use of Monio::writeIncrements from LFRic-Lite.

Current test outputs: http://fcm1/cylc-review/taskjobs/punderwo/?suite=monio_increment_write_05

The work in MONIO for use of Monio::readIncrements will come next, but the work in LFRic-Lite to use both of these functions will appear in a single PR.

Copy link
Contributor

@mo-joshuacolclough mo-joshuacolclough left a comment

Choose a reason for hiding this comment

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

Haven't tested myself but looks good. Had the odd comment (mostly formatting)

src/monio/AtlasWriter.cc Show resolved Hide resolved
src/monio/UtilsAtlas.cc Show resolved Hide resolved
Comment on lines -166 to +167
if (fieldMetadata.lfricWriteName != "TO BE DERIVED" &&
fieldMetadata.lfricWriteName != "TO BE IMPLEMENTED") {
if (fieldMetadata.copyFirstLevel == true) {
atlas::util::Config atlasOptions = atlas::option::name(fieldMetadata.lfricWriteName) |
if (writeName != consts::kToBeDerived && writeName != consts::kToBeImplemented) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cheers!

Comment on lines +215 to +222
int getDataSize(const atlas::Field& field) {
std::vector<int> dimVec = field.shape();
int size = 1;
for (const auto& dim : dimVec) {
size *= dim;
}
return size;
}
Copy link
Contributor

@mo-joshuacolclough mo-joshuacolclough Sep 13, 2023

Choose a reason for hiding this comment

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

Think this includes halo points in the count - is this what you want? Is it for global fields only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a very good spot. I didn't want the ghost points, I don't think. Let me play with this. There'll be an update on this, I'm sure. Thank you!

Copy link
Collaborator Author

@phlndrwd phlndrwd Sep 13, 2023

Choose a reason for hiding this comment

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

Ah yeah. I checked, @mo-joshuacolclough. I am using it with global fields only. So the current implementation is what I need. Glad you made me think about that though!

I'll rename the function to make it clear. Otherwise I'll probably forget later on!

src/monio/UtilsAtlas.h Show resolved Hide resolved
@phlndrwd phlndrwd merged commit 79daa56 into develop Sep 13, 2023
2 checks passed
@phlndrwd
Copy link
Collaborator Author

phlndrwd commented Sep 13, 2023

Thanks @mo-joshuacolclough. The change to UtilsAtlas::getDataSize will appear in the next PR. It just means that currently it will be temporarily using more data than it needs.

@phlndrwd phlndrwd deleted the feature/increment_write branch September 21, 2023 13:13
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.

2 participants