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 support for Enzo MHDCT fields #1438

Merged
merged 7 commits into from Jun 2, 2017

Conversation

ngoldbaum
Copy link
Member

This adds support for Enzo MHDCT fields, following the support @atmyers added for nodal WarpX data.

The implementation is relatively straightforward. I just needed to add support for I/O on these fields following more or less what the boxlib frontend does. I also needed to teach the enzo frontend which fields are expected to be nodal and what their expected nodal flags are. In principle we could read these data off disk like the boxlib frotend does but as far as I'm aware that data is currently not written out by Enzo so it needs to be hardcoded in the Enzo frontend.

While working on this I noticed a pattern where we do something like:

if selector.__class__.__name__ == "GridSelector":

many places in the codebase. Since this is arcane and wordy, I decided to globally replace it with

isinstance(selector, GridSelector)

which is a bit less arcane. Let me know if you'd prefer to have that change as a separate pull request.

@ngoldbaum ngoldbaum requested a review from atmyers June 1, 2017 20:29
direction, but nodal in the other two, i.e. it lives on the four cell edges that
are normal to the z direction.

..code-block:: python
Copy link
Member

Choose a reason for hiding this comment

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

There should be a space between .. and code-block. Also empty line right afterwards. Could you also fix the WarpX snippet while you're at it?

.. code-block:: python

ad = ds.all_data()
print(ds.field_info[('enzo', 'Ex')].nodal_flag)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if it's really important but if do just import, load, all_data the index won't be created at this point and it will fail with AttributeError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, I thought ds.all_data() created the index.

if selector.__class__.__name__ == "GridSelector":
if data is None:
continue
if selector.__class__ is GridSelector and field not in nodal_fields:
Copy link
Member

Choose a reason for hiding this comment

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

Why not isinstance(selector, GridSelector) ?

nodal_flag = self.ds.field_info[field].nodal_flag
dims = obj.ActiveDimensions[::-1].copy()
if np.any(nodal_flag):
dims += nodal_flag[::-1]
Copy link
Member

Choose a reason for hiding this comment

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

Since nodal_flag is [0,0,0] by default, can we do that unconditionally?

@matthewturk
Copy link
Member

This looks good, but the way the nodal flags are set inside a try/except for KeyError (which contains two possible KeyErrors) feels a bit awkward. I'd prefer to supply a default value and skip the t/e.

@matthewturk matthewturk merged commit e7b386a into yt-project:master Jun 2, 2017
@ngoldbaum ngoldbaum deleted the enzo-mhdct-fields branch June 16, 2017 16:36
matthewturk added a commit to matthewturk/yt that referenced this pull request Apr 17, 2018
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.

None yet

4 participants