Load FOF catalogues & Cosmology Metadata Changes #190

robjmcgibbon · 2024-06-18T15:34:25Z

Allow swiftsimio to load FOF files. I've made the SWIFTDataset class more generic, so it now has a filetype attribute (currently can be snapshot or FOF). Instead of storing an int for each particle type it now saves the names of the groups within the hdf5 that we want to load ('PartType{i}' for snapshots, and 'Groups' for fof files).

This seems to work for loading both snapshots and fof files. I'll have a look at the tests next.

JBorrow · 2024-06-20T13:57:16Z

swiftsimio/metadata/particle/particle_types.py

-    0: "gas",
-    1: "dark_matter",
-    2: "boundary",
-    3: "sinks",
-    4: "stars",
-    5: "black_holes",
-    6: "neutrinos",
+    "PartType0": "gas",
+    "PartType1": "dark_matter",
+    "PartType2": "boundary",
+    "PartType3": "sinks",
+    "PartType4": "stars",
+    "PartType5": "black_holes",
+    "PartType6": "neutrinos",
 }

 particle_name_class = {
-    0: "Gas",
-    1: "DarkMatter",
-    2: "Boundary",
-    3: "Sinks",
-    4: "Stars",
-    5: "BlackHoles",
-    6: "Neutrinos",
+    "PartType0": "Gas",
+    "PartType1": "DarkMatter",
+    "PartType2": "Boundary",
+    "PartType3": "Sinks",
+    "PartType4": "Stars",
+    "PartType5": "BlackHoles",
+    "PartType6": "Neutrinos",
 }

 particle_name_text = {
-    0: "Gas",
-    1: "Dark Matter",
-    2: "Boundary",
-    3: "Sinks",
-    4: "Stars",
-    5: "Black Holes",
-    6: "Neutrinos",
+    "PartType0": "Gas",
+    "PartType1": "Dark Matter",
+    "PartType2": "Boundary",
+    "PartType3": "Sinks",
+    "PartType4": "Stars",
+    "PartType5": "Black Holes",
+    "PartType6": "Neutrinos",


Why did you change this?

I changed present_particle_types to return "PartType{i}" rather than integers

swiftsimio/swiftsimio/reader.py

Line 652 in 143d65f

return [f"PartType{i}" for i in types]

So I could either change this file, or else I'd have to convert back to an integer when getting the particle name

Are we not able to remove these for modern snapshots? As they contain the particle type names and fields named appropriately..? I'd like to use those so we don't have to add/handle additional or renamed particle types manually.

robjmcgibbon · 2024-06-20T14:35:50Z

I'm now loading SOAP catalogues as well. The groups which we want to load are stored in the SOAP metadata. A slight complication is that we have subgroups, i.e. the 200c halos properties are stored in the group /SO/200_crit, and the x projection of the 100kpc aperture are stored in the group /ProjectedAperture/100kpc/projx. It would be nice to try and preserve this structure so we would access the group with projected_aperture.100kpc.proj_x, but I don't know of any way in python to have attributes that start with numbers, and I don't want to go with projected_aperture.kpc100

Masking seems to work for the SOAP catalogues. The FOF catalogues aren't sorted so we can't apply masks to them.

robjmcgibbon · 2024-06-21T13:03:46Z

SOAP contains some additional metadata for each dataset that we need to read in.

SOAP files are first generated and then compressed by a separate script. This means we save the type of compression to apply to the dataset, and whether the compression has been applied.
We want all the arrays in SOAP to be the same length (the number of subhalos). However, we don't calculate all quantities for all objects, e.g. we have some quantities that are only calculated for subhalos with 100 bound gas particles. This means the arrays can have a large number of zeros. I'm now reading in the mask applied and adding that information to the array description.

kyleaoman · 2024-07-17T22:36:09Z

Hi Rob, little thing that I noticed trying to experiment a bit with this, if I install normally (pip install swiftsimio/) I get:

>>> import swiftsimio as sw
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/cosma/home/durham/dc-oman1/.virtualenv/ssio/lib/python3.12/site-packages/swiftsimio/__init__.py", line 1, in <module>
    from .reader import *
  File "/cosma/home/durham/dc-oman1/.virtualenv/ssio/lib/python3.12/site-packages/swiftsimio/reader.py", line 14, in <module>
    from swiftsimio import metadata
  File "/cosma/home/durham/dc-oman1/.virtualenv/ssio/lib/python3.12/site-packages/swiftsimio/metadata/__init__.py", line 9, in <module>
    from .soap import soap_types
ModuleNotFoundError: No module named 'swiftsimio.metadata.soap'

If I install via link (pip install -e swiftsimio/), I don't have this problem. Should just be a matter of including in the pyproject.toml file, I think.

JBorrow · 2024-07-18T00:35:31Z

Gotta add the submodule to the list in the pyproject.toml

kyleaoman · 2024-07-25T13:45:56Z

When you get to the stage of looking at the masking, could I make a request? I know that it was planned to be able to apply spatial masks like we do with particle data. Could we also have an option to read a user-defined slice of the subhalo table (might have to handle subhalo and fof tables separately? can't remember the structure in SOAP hdf5 files and the sample seems to have moved/vanished). It's possible that there's nothing extra to be done since with a spatial_only=False mask you can do virtually anything, just want to flag it as something to check. My use case is that I want to be able to specify that I'm interested in subhalo 12345 (using a mask makes sense for this), and then when I do:

data = sw.load(...)
data.bound_subhalo.enclose_radius  # or whatever other field

I would like to read only the single row of interest. I said slice above because there could be use cases (hypothetically) to read a range or other slice, and it's the same amount of work to implement this as masking to a single row. Like I said I don't think much is needed because the masking tools just about support this already, but I guess the code needs to understand what's an array of subhalo data vs fof data vs anything else.

robjmcgibbon · 2024-07-25T14:22:43Z

When you get to the stage of looking at the masking, could I make a request? I know that it was planned to be able to apply spatial masks like we do with particle data. Could we also have an option to read a user-defined slice of the subhalo table (might have to handle subhalo and fof tables separately? can't remember the structure in SOAP hdf5 files and the sample seems to have moved/vanished). It's possible that there's nothing extra to be done since with a spatial_only=False mask you can do virtually anything, just want to flag it as something to check. My use case is that I want to be able to specify that I'm interested in subhalo 12345 (using a mask makes sense for this), and then when I do:
data = sw.load(...)
data.bound_subhalo.enclose_radius  # or whatever other field
I would like to read only the single row of interest. I said slice above because there could be use cases (hypothetically) to read a range or other slice, and it's the same amount of work to implement this as masking to a single row. Like I said I don't think much is needed because the masking tools just about support this already, but I guess the code needs to understand what's an array of subhalo data vs fof data vs anything else.

Sorry about moving the example catalogue, they're now at /cosma8/data/dp004/dc-mcgi1/FLAMINGO/Runs/L1000N1800/DMO_FIDUCIAL/SOAP/HBTplus

I can make a new method for SWIFTMask called something like constrain_index. I don't think it should matter if it's a FOF/SOAP/snapshot file, they should all work the same way.

kyleaoman · 2024-07-25T14:28:18Z

Makes sense. In EAGLE we had /FOF/... datasets and /Subhalo/... datasets coexisting in the same file, couldn't remember offhand if this was the case in SOAP, which would require a bit more logic, but looking now, it's not, so it should indeed be a minimal addition. Thanks!

robjmcgibbon · 2024-09-12T18:02:37Z

@JBorrow update on the current status of this in case you have time to look it over before the meeting next week

This change adds an attribute to the SWIFTMetadata class called "filetype" which can be either "snapshot", "FOF", or "SOAP". I haven't changed much when it comes to reading in the files, everything has just been made more generic so instead of requiring the data to be in groups named "PartTypeX", the groups can have any names. Masking appears to be working for the "SOAP" filetype, it's not supported for "FOF" since SWIFT does not output spatially sorted catalogues.

SWIFT has added two new attributes to it's units metadata - one to indicate if a value is stored as physical, another to indicate if a value can be transformed between comoving and physical. This was motivated by quantities such as BirthDensities, which it doesn't make sense to convert. These attributes are now read in when reading a file, and the cosmo_array has a valid_transform attribute, which throws an error if someones tries to convert something they shouldn't.

Things to be done:

Add tests for the "FOF" and "SOAP" filetypes
Add documentation for the "FOF" and "SOAP" filetypes
Implement a writer for "FOF" and "SOAP" (not sure this is needed, other than being helpful for tests)
Decide what to do with swiftsimio functionality that only make sense for snapshots (e.g. visualisation)

JBorrow · 2024-09-12T18:33:49Z

I am not sure that there's any functionality that only makes sense for snapshots. Visualisation is still useful with halo catalogues; for instance I use that in a hacky way right now to create cross spectra with haloes...

JBorrow

I would really like to see a comprehensive test suite here to make sure we're not missing anything.

JBorrow · 2024-09-16T13:36:37Z

swiftsimio/__init__.py

+        if filetype == "snapshot":
+            return SWIFTSnapshotWriter(*args, **kwargs)
+        # TODO implement other writers
+        # elif filetype == '


Is it likely that anyone will want to write their own catalogue files with swiftsimio? Ideally we would just leave this feature alone until we are ready to make that change.

Given that people have to choose their 'file type' anyway, why don't we just make them choose the class they want to instantiate? We don't need to abstract this away.

JBorrow · 2024-09-16T13:38:46Z

swiftsimio/masks.py

@@ -128,7 +139,7 @@ def _unpack_cell_metadata(self):

    def constrain_mask(
        self,
-        ptype: str,
+        group_name: str,


This is fine, but in general I don't like _name as an addition to the variable name. Given that we know its type is string, group is obviously a name.

JBorrow · 2024-09-16T13:42:19Z

swiftsimio/metadata/particle/particle_types.py

-    0: "gas",
-    1: "dark_matter",
-    2: "boundary",
-    3: "sinks",
-    4: "stars",
-    5: "black_holes",
-    6: "neutrinos",
+    "PartType0": "gas",
+    "PartType1": "dark_matter",
+    "PartType2": "boundary",
+    "PartType3": "sinks",
+    "PartType4": "stars",
+    "PartType5": "black_holes",
+    "PartType6": "neutrinos",
 }

 particle_name_class = {
-    0: "Gas",
-    1: "DarkMatter",
-    2: "Boundary",
-    3: "Sinks",
-    4: "Stars",
-    5: "BlackHoles",
-    6: "Neutrinos",
+    "PartType0": "Gas",
+    "PartType1": "DarkMatter",
+    "PartType2": "Boundary",
+    "PartType3": "Sinks",
+    "PartType4": "Stars",
+    "PartType5": "BlackHoles",
+    "PartType6": "Neutrinos",
 }

 particle_name_text = {
-    0: "Gas",
-    1: "Dark Matter",
-    2: "Boundary",
-    3: "Sinks",
-    4: "Stars",
-    5: "Black Holes",
-    6: "Neutrinos",
+    "PartType0": "Gas",
+    "PartType1": "Dark Matter",
+    "PartType2": "Boundary",
+    "PartType3": "Sinks",
+    "PartType4": "Stars",
+    "PartType5": "Black Holes",
+    "PartType6": "Neutrinos",


Are we not able to remove these for modern snapshots? As they contain the particle type names and fields named appropriately..? I'd like to use those so we don't have to add/handle additional or renamed particle types manually.

JBorrow · 2024-09-16T13:43:41Z

swiftsimio/metadata/soap/soap_types.py

+# Describes the conversion of hdf5 groups to names
+def get_soap_name_underscore(group: str) -> str:
+    soap_name_underscores = {
+        "BoundSubhalo": "bound_subhalo",
+        "InputHalos": "input_halos",
+        "InclusiveSphere": "inclusive_sphere",
+        "ExclusiveSphere": "exclusive_sphere",
+        "SO": "spherical_overdensity",
+        "SOAP": "soap",
+        "ProjectedAperture": "projected_aperture",
+    }
+    split_name = group.split("/")
+    split_name[0] = soap_name_underscores[split_name[0]]
+    return "_".join(name.lower() for name in split_name)


Is there a reason we do this and not handle the conversion in the same way as field names?

JBorrow · 2024-09-16T13:44:27Z

swiftsimio/objects.py

    """

+    # TODO:


What is TODO

JBorrow · 2024-09-16T13:55:03Z

swiftsimio/reader.py

+            try:
+                # SOAP catalogues can be compressed/uncompressed
+                is_compressed = dataset.attrs["Is Compressed"]
+            except KeyError:
+                is_compressed = True
            try:


is_compressed = dataset.attrs.get("Is Compressed", True)

JBorrow · 2024-09-16T13:55:12Z

swiftsimio/reader.py

+            except AttributeError:
+                # Compression is saved as str not bytes
+                comp = dataset.attrs["Lossy compression filter"]
            except KeyError:
                # Can't load compression string!
                comp = "No compression info available"


JBorrow · 2024-09-16T13:57:02Z

swiftsimio/reader.py

+            try:
+                physical = dataset.attrs["Value stored as physical"][0] == 1
+            except:
+                physical = False
+            return physical


Don't like this at all; never have non-specific exceptions. Can you do:

return bool(dataset.attrs.get("Value stored as physical", [1])[0])

JBorrow · 2024-09-16T13:57:17Z

swiftsimio/reader.py

+        def get_valid_transform(dataset):
+            try:
+                valid_transform = (
+                    dataset.attrs["Property can be converted to comoving"][0] == 1
+                )
+            except:
+                valid_transform = True
+            return valid_transform


Same concern

JBorrow · 2024-09-16T13:58:17Z

swiftsimio/reader.py

+    if group_metadata.metadata.filetype == "snapshot":
+        group_nice_name = metadata.particle_types.particle_name_class[group]
+    elif group_metadata.metadata.filetype == "FOF":
+        group_nice_name = "FOFGroups"
+    elif group_metadata.metadata.filetype == "SOAP":
+        group_nice_name = metadata.soap_types.get_soap_name_nice(group)


Why if we wrote all this code are we doing this three different ways for three catalogues that we wrote..?

Load fof catalogues refactor

…ftsimio into load_fof_catalogues

Make reader more generic

143d65f

JBorrow reviewed Jun 20, 2024

View reviewed changes

Load SOAP catalogues

3d0187a

Read all SOAP dataset attributes

9ca0d29

robjmcgibbon added 2 commits June 21, 2024 14:05

Formatter

4ac1cba

Add new SWIFT attributes

8b94a0b

robjmcgibbon and others added 11 commits August 2, 2024 10:05

Squash bug registering field names

0e6c14c

Add swiftsimio.metadata.soap to module list.

186968a

Bugfixes following changes in particle_types.py

8b8937f

Add missing visualisation.smoothing_length to pyproject.toml.

43fe3b3

Implement an option to mask a SOAP catalogue down to a single row.

3292d56

Merge branch 'master' into load_fof_catalogues

90f9e4c

Fix some tests

6a1675b

Comment out failing test

7a48a7d

Format

b4d7649

Add InvalidConversionError

2ac42bf

Format

64b8d94

JBorrow reviewed Sep 16, 2024

View reviewed changes

JBorrow and others added 3 commits September 18, 2024 16:58

Added refactoring

695f7e1

Added test loading a SOAP catalogue

6b26291

Nice names

988dc28

JBorrow and others added 13 commits September 18, 2024 17:15

Packaging changes

276f4fe

Added optional named column metadata

828d649

Add shared cell counts

aca5048

Formatting and cleanup

e6b4a62

Merge pull request #195 from SWIFTSIM/load_fof_catalogues_refactor

d8ccdae

Load fof catalogues refactor

Drop 3.8 and 3.9

c7cd6b0

Merge branch 'load_fof_catalogues' of https://github.com/swiftsim/swi…

c325047

…ftsimio into load_fof_catalogues

Fix broken test and remove unused writer object

77e361c

Add Writer back

ba2491e

Use uv for package installation

a472d71

Use system?

3188823

Formatting

fab3d14

where does system go???

155e626

JBorrow changed the title ~~Load FOF catalogues~~ Load FOF catalogues & Cosmology Metadata Changes Sep 19, 2024

JBorrow added 7 commits September 19, 2024 10:08

Remove pip caching

44e22e8

Go back to pip...

68c0382

Merge branch 'master' into load_fof_catalogues

66ea952

Fix incorrect import now that particle datasets have been renamed

d184d0b

Fix volume render test

43582fc

Resolve warnings when accessing internal numpy objects

2dd0eb8

Added SOAP docs

45e2b8f

JBorrow marked this pull request as ready for review September 19, 2024 08:34

JBorrow merged commit ac05b8d into master Sep 19, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load FOF catalogues & Cosmology Metadata Changes #190

Load FOF catalogues & Cosmology Metadata Changes #190

robjmcgibbon commented Jun 18, 2024

JBorrow Jun 20, 2024

robjmcgibbon Jun 20, 2024

JBorrow Sep 16, 2024

robjmcgibbon commented Jun 20, 2024

robjmcgibbon commented Jun 21, 2024

kyleaoman commented Jul 17, 2024

JBorrow commented Jul 18, 2024

kyleaoman commented Jul 25, 2024

robjmcgibbon commented Jul 25, 2024

kyleaoman commented Jul 25, 2024

robjmcgibbon commented Sep 12, 2024

JBorrow commented Sep 12, 2024

JBorrow left a comment

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

JBorrow Sep 16, 2024

Load FOF catalogues & Cosmology Metadata Changes #190

Load FOF catalogues & Cosmology Metadata Changes #190

Conversation

robjmcgibbon commented Jun 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robjmcgibbon commented Jun 20, 2024

robjmcgibbon commented Jun 21, 2024

kyleaoman commented Jul 17, 2024

JBorrow commented Jul 18, 2024

kyleaoman commented Jul 25, 2024

robjmcgibbon commented Jul 25, 2024

kyleaoman commented Jul 25, 2024

robjmcgibbon commented Sep 12, 2024

JBorrow commented Sep 12, 2024

JBorrow 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment