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

Ignore FieldPerps when loading datasets #37

Merged
merged 2 commits into from
Aug 13, 2019
Merged

Conversation

johnomotani
Copy link
Collaborator

BOUT++ 4.3 will be able to save FieldPerps to dump files. They will not be consistent across all processors, so require some special handling. In principle we could do something smart (see below), but for now just ignore all FieldPerps so that we can load data-sets that include them and read other fields.

FieldPerps are saved with a global y-index as an attribute, and FieldPerps representing physical quantities (e.g. the flux at a divertor target) should have that index set to either a single, consistent value (on the processes where the FieldPerp actually is) or a negative value (on all other processes). So with some special handling they could be loaded sensibly. There is no global checking, so a FieldPerp could for example be written from all processes, but then it cannot represent a single physical quantity and we should leave it to the user to handle separately.

@codecov-io
Copy link

codecov-io commented Jul 16, 2019

Codecov Report

Merging #37 into master will decrease coverage by 0.15%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
- Coverage   51.85%   51.69%   -0.16%     
==========================================
  Files           8        8              
  Lines         351      354       +3     
  Branches       63       65       +2     
==========================================
+ Hits          182      183       +1     
- Misses        153      154       +1     
- Partials       16       17       +1
Impacted Files Coverage Δ
xbout/load.py 78.94% <33.33%> (-1.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78a8b9f...abd91b7. Read the comment docs.

Copy link
Collaborator

@TomNicholas TomNicholas left a comment

Choose a reason for hiding this comment

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

I can't say I really understand how FieldPerps work. Is this related to boutproject/BOUT-dev#1641?

@@ -181,6 +181,12 @@ def _trim(ds, ghosts={}, keep_guards=True):

trimmed_ds = trimmed_ds.drop(_BOUT_TIMING_VARIABLES, errors='ignore')

# Ignore FieldPerps for now
for name in trimmed_ds:
if (trimmed_ds[name].dims == ('x', 'z')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do FieldPerps not have any identifying features other than their literal dimensions? Like an attributes flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FieldPerps do have an attribute yindex_global which other fields don't have, but I think at the moment the convention for identifying Field3D, Field2D, FieldPerp in netCDF files is by their dimensions. In the current setup this is unambiguous; are you thinking it'll cause problems when adding 'physical' coordinates, etc.? It would be easy to add an attribute like bout_type - this is already done in BOUT++ for the HDF5 input/output because HDF5 variables don't have dimensions that we could use to identify them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay that seems fine for now then

@johnomotani
Copy link
Collaborator Author

FieldPerp I/O was implemented in this PR boutproject/BOUT-dev#1699, where there's a bit of discussion. Closely related to boutproject/BOUT-dev#1641 - this is another use-case to consider.

@@ -181,6 +181,12 @@ def _trim(ds, ghosts={}, keep_guards=True):

trimmed_ds = trimmed_ds.drop(_BOUT_TIMING_VARIABLES, errors='ignore')

# Ignore FieldPerps for now
for name in trimmed_ds:
if (trimmed_ds[name].dims == ('x', 'z')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay that seems fine for now then

@TomNicholas TomNicholas merged commit 2cf72ec into master Aug 13, 2019
@johnomotani johnomotani deleted the ignore-fieldperp branch August 13, 2019 14:37
johnomotani pushed a commit that referenced this pull request Dec 11, 2019
Ignore FieldPerps when loading datasets
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