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

Add a simple parallel write example #21

Merged
merged 5 commits into from
May 22, 2024

Conversation

svadams
Copy link
Collaborator

@svadams svadams commented May 22, 2024

Parallel write example with synthetic data showing output at a user-defined interval. Some basic unit tests have been added to check the file is created and has the correct number of data points in it

@svadams svadams requested a review from mo-marqh May 22, 2024 09:29
@svadams svadams force-pushed the paralle1_write_example branch from 913500e to e02c61e Compare May 22, 2024 10:39
Copy link
Member

@mo-marqh mo-marqh left a comment

Choose a reason for hiding this comment

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

hi @svadams

this looks good,

i'd like to see this running with 2 clients and 2 servers for parallelism demonstration.

other than that there a a few v minor changes suggested in this review, which you can adopt or push back on as you see fit.

many thanks
mark

# run the compiled Fortran XIOS programme
with open('{}/xios.xml'.format(self.test_dir)) as cxml:
print(cxml.read(), flush=True)
self.run_mpi_xios()
Copy link
Member

Choose a reason for hiding this comment

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

this test is configured for parallel writing, but there is only 1 client and 1 server.

i think that this still runs fine if both nclients and nservers is set to 2, so dual parallel

please may you update this line to
self.run_mpi_xios(nclients=2, nservers=2)


# Check the expected output file exists
runfile_1 = '{}/{}'.format(self.test_dir, outputfile_1)
assert(os.path.exists(runfile_1))
Copy link
Member

Choose a reason for hiding this comment

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

prefer self.assertTrue (to benefit from python's UnitTest assertion handling

file_1_data = rootgrp['global_field_1']

# Check file has 10 times
assert(file_1_data.shape[0] == 10)
Copy link
Member

Choose a reason for hiding this comment

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

prefer self.assertTrue (to benefit from python's UnitTest assertion handling

if not np.allclose(result, expected, rtol=self.rtol):
# print message for fail case,
# as expected failures do not report msg.
print(msg)
Copy link
Member

Choose a reason for hiding this comment

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

i think that lines #48-#51 are not used here, as there are no expected failures being handled

i suggest removing

@mo-marqh mo-marqh merged commit 3d51cde into MetOffice:main May 22, 2024
4 checks passed
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