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 new feature ‘custom diagnostics’ to Fluid2D #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kegelkugel
Copy link
Collaborator

Custom diagnostics allow to store any information that can be extracted
from the model state in the diagnostics file. With this, we can provide
a high frequency output of a diagnostics we are interested in without
creating a huge history file. This may make the diagnostics file more
useful for a wider range of Fluid2D users.

An example is provided where custom diagnostics are used to measure the
drift of a vortex that he experiences due to the beta-effect on a
rotating planet.

Note: the experiment only works correctly with the fixed QG model
(see pull request "Fix QG model")

Custom diagnostics allow to store any information that can be extracted
from the model state in the diagnostics file.  With this, we can provide
a high frequency output of a diagnostics we are interested in without
creating a huge history file.  This may make the diagnostics file more
useful for a wider range of Fluid2D users.

An example is provided where custom diagnostics are used to measure the
drift of a vortex that he experiences due to the beta-effect on a
rotating planet.

Note: the experiment only works correctly with the fixed QG model
(see pull request "Fix QG model")
@kegelkugel
Copy link
Collaborator Author

kegelkugel commented Jun 6, 2020

Possibly unexpected results in multi-core runs

When the example experiment is run on multi-cores, the position of the vortex is not calculated correctly, because the core 0 that is responsible for writing diagnostics only sees one part of the domain. Before this pull request is merged, one should pose the question of how to handle multi-core runs.

@kegelkugel
Copy link
Collaborator Author

Comments on the multi-core situation

The way custom diagnostics are implemented in this pull request works when Fluid2D is run on a single core, which is already a gain. In a multi-core setting, however, the example does not create the expected output for the reason stated above. I see three options to handle this situation:

  • add a warning to the corresponding code to make the user aware of this limitation. Pros: no increased work; easy solution for the most common case of a single core run. Cons: result depends on the number of cores used; custom diagnostics might not be useful on multicores.

  • let the custom diagnostics be calculated on every single core and store the result of each computation in different fields. That means, if the simulation is run on a single core, it produces the custom diagnostics "x_pos", if it is run on two cores it produces "x_pos_0" and "x_pos_1"; it is then up to the user to extract the information he's interested in. Pros: rather easy to implement; gives full information. Cons: file size increases with the number of cores; additional work for the user to post-process the data.

  • make some implementation similar to mpitools.local_to_global, which is already used in the calculation of the normal diagnostics. Pro: best result. Con: difficult to implement for both the developers and for the user, because the user has to specify how to merge the results calculated on the different cores; for the given example, the merge would mean: take the position calculated by the core with the maximum vorticity. Another con: might make the single-core case more difficult than necessary.

I prefer option 1, but it is up to the reviewer to decide.

@kegelkugel kegelkugel added the work in progress Do not merge a pull request with this label into master. label Jun 7, 2020
@kegelkugel kegelkugel removed the work in progress Do not merge a pull request with this label into master. label Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants