New fields for Arepo and Gadget frontends #3258

jzuhone · 2021-05-13T18:49:03Z

PR Summary

This PR adds new fields for both the Arepo and Gadget frontends.

For Arepo: adds the "CosmicRaySpecificEnergy" field and "PassiveScalars"
For Gadget: adds specifications to support Magneticum halos (http://www.magneticum.org) and known fields
For Gadget: If we already have a temperature field in the dataset, there is no need to create a derived field for it. This was technically a bug.

EDIT: This got more complicated. It turns out that if you naively run sph_answer on the Magneticum dataset that you get an error because you project a sphere at the domain center with no data in it (the data is confined to a small region of the domain). The error will need to be fixed in a separate PR.

It would have been simple to set a bounding box around the data, but when I tried this it took forever to construct the index. So I did a simple (and probably useful in the future thing) and allowed the sphere in sph_answer and nbody_answer to be set to some alternate center.

PR Checklist

New features are documented, with docstrings and narrative docs
Adds a test for any bugs fixed. Adds tests for new features.

…ements actually are.

jzuhone · 2021-05-13T19:43:42Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

…get binary files

jzuhone · 2021-05-14T13:33:00Z

I added more doc edits--it turns out we had a number of things in Gadget and Arepo data that needed explaining in the docs.

For the tests to pass this needs the dataset which will be part of https://yt-project.org/data once PR yt-project/website#96 is merged (ping @Xarthisius) and I upload new answers.

neutrinoceros · 2021-05-15T13:04:31Z

yt/frontends/gadget/tests/test_outputs.py

@@ -124,3 +125,25 @@ def test_bigendian_field_access():
    ds = data_dir_load(BE_Gadget)
    data = ds.all_data()
    data["Halo", "Velocities"]
+
+
+mag_fields = OrderedDict(


quick note: since Python 3.6, builtin dict objects are also ordered. At the time it was deemed an implementation detail, but it's been embraced as a feature since.
Some minor differences persist (see https://stackoverflow.com/questions/34305003/difference-between-dictionary-and-ordereddict), but it's doubtful we need them here

So if you look at the top of the file:

yt/yt/frontends/gadget/tests/test_outputs.py

Lines 27 to 35 in 0c0ad20

# This maps from field names to weight field names to use for projections

iso_fields = OrderedDict(

[

(("gas", "density"), None),

(("gas", "temperature"), None),

(("gas", "temperature"), ("gas", "density")),

(("gas", "velocity_magnitude"), None),

]

)

We have another one of these in there, which I copied to make the new one below. For some reason there are two ("gas","temperature") entries in here--OrderedDict simply replaces the first one with the second one silently without complaining. When I tried to change this to a dict the pre-commit bot complained.

So the real question is if sph_answer is expecting us to try projections with and without the weight field for ("gas","temperature"). Does anyone know (I can look myself, but if anyone knows please comment)? @matthewturk @brittonsmith @chummels

It seems to me taking an unweighted projection of ("gas", "temperature") would be useless and non-sensical, so I don't think sph_answer should do that.

It probably is expecting that. A non-weighted temperature projection is generally not useful, but it can also serve as a proxy for testing in situations where the temperature itself is a derived field (rather than intrinsic) and we want to test the machinery of unweighted temperature projections. I don't know that we need to keep it in this instance, but I don't think we should always require that answer tests always map 1:1 to useful quantities.

matthewturk

LGTM; @jzuhone what is pending still?

jzuhone · 2021-05-18T13:17:07Z

@matthewturk need to fix the pre-commit issue, update test answers

jzuhone · 2021-05-18T18:06:25Z

@Xarthisius it looks like I screwed up the answer testing--can we remove local_gadget_007 on the build box?

Xarthisius · 2021-05-18T18:11:26Z

@yt-fido test this please

jzuhone · 2021-05-18T18:43:06Z

@Xarthisius thanks!

jzuhone · 2021-05-19T11:14:33Z

This is finally ready.

neutrinoceros · 2021-05-19T11:48:13Z

yt/sample_data_registry.json

@@ -259,6 +259,15 @@
    "load_name": "plt00015",
    "url": "https://yt-project.org/data/Laser.tar.gz"
  },
+  "MagneticumCluster.tar.gz": {


in order for this to integrate with yt.load_sample, you'll need to update https://github.com/yt-project/website/blob/master/data/datafiles.json

@neutrinoceros This was done already (you looked at the PR): yt-project/website#96

I didn't realise it was the same one. In any case I've tested this branch and it currently doesn't work

import yt yt.load_sample("MagneticumCluster")

Do you have a traceback or something for more information?

For reference, this is what the traceback looked like (rather cryptic if you ask me) I made it much more informative for next time this kind of accident happens in #3277

yt/sample_data_registry.json

Co-authored-by: Clément Robert <cr52@protonmail.com>

chummels

In general this looks good; I have just a few comments inline. Thanks for putting some love into the AREPO/Gadget frontends. It's good that they're getting a bit more attention with the yt4 refactor.

chummels · 2021-05-19T21:40:18Z

doc/source/examining/loading_data.rst

+and aliased to fields with the naming convention ``"PassiveScalars_XX"`` where
+``XX`` is the number of the passive scalar array, e.g. ``"00"``, ``"01"``, etc.
+
+HDF5 snapshots will be detected as Arepo data if they have the ``"GFM_Metals"``


Nice! Thanks for including this, @jzuhone !

chummels · 2021-05-19T21:48:52Z

yt/frontends/gadget/fields.py

+        This gets used with the magneticum_box2_hr format
+        as defined in the gadget_field_specs in frontends/gadget/definitions.py
+        """
+        metal_names = ["He", "C", "Ca", "O", "N", "Ne", "Mg", "S", "Si", "Fe", "Ej"]


What's Ej? Is that ejected metals or something?

Yes, that's correct--that's the name that the magneticum folks use for it.

chummels · 2021-05-19T21:50:53Z

yt/frontends/gadget/fields.py

+            )
+
+        # hydrogen fraction and density
+        def _h_fraction(field, data):


Oh this is interesting, calculating the H fraction as the remainder when you take away all the other elements. I think this will only work if Ej is distinct from all of the other elements present, though. Can you confirm?

Yes, that's the way they are defined in magneticum.

chummels · 2021-05-19T21:58:04Z

yt/frontends/gadget/fields.py

+
+                def _temperature(field, data):
+                    # Assume cosmic abundances
+                    x_H = _primordial_mass_fraction["H"]


Maybe it's better to look on disk and see if there is already an H_fraction field and use that instead of assuming primordial?

Hmm. we need to think about this one a bit more carefully. I didn't change the field definition--I just moved it within a new if-then block. But I see what you mean. I'll try changing it, but I suspect it will break answer tests.

@jzuhone pointed me to the TNG documentation where they themselves are calculating temperature using this formula, with primordial hydrogen abundances. So we should mimic that behavior and I retract my point above.

jzuhone added 9 commits May 13, 2021 14:36

Support PassiveScalars in Arepo

1412564

Add cosmic ray energy fields

401203a

Magneticum field definitions. Still need to figure out what the 11 el…

f9011fb

…ements actually are.

Get the metal fractions right

9eb1014

Bug fixes

b3da6f4

Bugfix

fe52279

Add new fields

16c6a40

Add other specs to docs

47922fd

Fix this function that I didn't want to change

b01624a

jzuhone added code frontends Things related to specific frontends new feature Something fun and new! labels May 13, 2021

Docs for passive scalars

a7847ee

pre-commit-ci bot and others added 5 commits May 13, 2021 19:45

[pre-commit.ci] auto fixes from pre-commit.com hooks

7c22701

for more information, see https://pre-commit.ci

New test for magneticum dataset

3554d03

Bugfix

77f212c

Switch the magneticum field spec back and add new docs on loading Gad…

251338e

…get binary files

Small doc edits

df06bba

neutrinoceros reviewed May 15, 2021

View reviewed changes

jzuhone added 3 commits May 15, 2021 13:15

Add the MagneticumCluster to the data registry

12b7064

I messed this up

b82093c

Merge branch 'main' into new_arepo_fields

57c8268

matthewturk approved these changes May 18, 2021

View reviewed changes

jzuhone added 3 commits May 18, 2021 09:25

Update docs

89d7a95

Update gadget tests

c8325c1

I got this wrong

35379bd

jzuhone added 3 commits May 18, 2021 21:02

Allow a center parameter to be specified here

0d4c406

Set the center to be max

e4ec743

Bump this again

5d3d326

neutrinoceros reviewed May 19, 2021

View reviewed changes

neutrinoceros mentioned this pull request May 19, 2021

load_sample: add a clear error message for datasets missing website registration #3277

Merged

neutrinoceros reviewed May 19, 2021

View reviewed changes

yt/sample_data_registry.json Outdated Show resolved Hide resolved

neutrinoceros reviewed May 19, 2021

View reviewed changes

yt/sample_data_registry.json Outdated Show resolved Hide resolved

Update yt/sample_data_registry.json

b2cfbfd

Co-authored-by: Clément Robert <cr52@protonmail.com>

chummels approved these changes May 19, 2021

View reviewed changes

chummels merged commit 7bbaa93 into yt-project:main May 20, 2021

jzuhone deleted the new_arepo_fields branch May 21, 2021 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New fields for Arepo and Gadget frontends #3258

New fields for Arepo and Gadget frontends #3258

jzuhone commented May 13, 2021 •

edited

Loading

jzuhone commented May 13, 2021

jzuhone commented May 14, 2021

neutrinoceros May 15, 2021

jzuhone May 15, 2021

chummels May 15, 2021

matthewturk May 15, 2021

matthewturk left a comment

jzuhone commented May 18, 2021

jzuhone commented May 18, 2021

Xarthisius commented May 18, 2021

jzuhone commented May 18, 2021

jzuhone commented May 19, 2021

neutrinoceros May 19, 2021

jzuhone May 19, 2021

neutrinoceros May 19, 2021

jzuhone May 19, 2021

neutrinoceros May 19, 2021

chummels left a comment

chummels May 19, 2021

chummels May 19, 2021

jzuhone May 20, 2021

chummels May 19, 2021

jzuhone May 20, 2021

chummels May 19, 2021

jzuhone May 20, 2021

chummels May 20, 2021

	# This maps from field names to weight field names to use for projections
	iso_fields = OrderedDict(
	[
	(("gas", "density"), None),
	(("gas", "temperature"), None),
	(("gas", "temperature"), ("gas", "density")),
	(("gas", "velocity_magnitude"), None),
	]
	)

New fields for Arepo and Gadget frontends #3258

New fields for Arepo and Gadget frontends #3258

Conversation

jzuhone commented May 13, 2021 • edited Loading

PR Summary

PR Checklist

jzuhone commented May 13, 2021

jzuhone commented May 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewturk left a comment

Choose a reason for hiding this comment

jzuhone commented May 18, 2021

jzuhone commented May 18, 2021

Xarthisius commented May 18, 2021

jzuhone commented May 18, 2021

jzuhone commented May 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chummels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzuhone commented May 13, 2021 •

edited

Loading