pynbody / tangos

The Agile Numerical Galaxy Organisation System
BSD 3-Clause "New" or "Revised" License
19 stars 13 forks source link

Adaptahop automatic detection properties #160

Closed Martin-Rey closed 2 years ago

Martin-Rey commented 2 years ago

Hi @apontzen,

This PR codes up a new PynbodyHandler for AdaptaHOP halo and sub halo catalogues. It imports most of the pre-computed properties by AdaptaHOp (such as the virial mass or center), through a slightly different logic than usual, as unlike other handlers, these properties are stored as a dictionary attached to each pynbody halo rather than in a stat file.

The handler also creates links between parent and child halos to navigate the relationship between halo and sub halos, although it currently only supports links between the first parent and its sub halos (i.e. subsub, subsubsubhalos etc are not tracked).

I am not entirely clear what the testing strategy is for new handlers, so far I have only verified carefully that the imported properties and links match between pynbody and Tangos. The import is quite fast on my local machine, taking about 100ms per halo to import all relevant quantities, which totals to ~5min per simulation output for a state-of-the-art zoom simulation.

In conjunction with https://github.com/pynbody/pynbody/pull/630/files and #159, this should provide a base pipeline for Tangos+pynbody+adaptaHOP significantly speeding up what we currently have.

Thanks to @cphyc for starting the base work and for reviewing the code. Any further comments welcome, this is the first time I touch this part of Tangos.

Best, Martin

Martin-Rey commented 2 years ago

@apontzen I think after further testing and cleaning up, @cphyc and I are now happy with this pull request (and the accompanying pynbody pull request for AdapatHOP https://github.com/pynbody/pynbody/pull/630/files).

As far as we can tell, the handler behaves as expected, allows the import of properties in about ~5min per snapshot for a typical modern zoom, and support linking and merger tree building.

Feel free to merge unless you have further comments!

apontzen commented 2 years ago

Thanks!