SWIFTSIM / HBTplus

HBTplus halo finder adapted for the FLAMINGO and COLIBRE simulations
0 stars 0 forks source link

Function to wrap coordinates based on boundary conditions #9

Closed VictorForouhar closed 3 months ago

VictorForouhar commented 5 months ago

Previously, HBT would not wrap coordinates after adding a shift vector to an origin point. This would lead to some subhaloes having positions outside of the box limits.

To fix this, I created a new function that wraps coordinates, and apply it to coordinates whenever these are based on adding a shift to a reference point.

VictorForouhar commented 5 months ago

There is another function that also fixes issues concerning coordinates beyond box size (inline HBTReal position_modulus(HBTReal x, HBTReal boxsize)), but I think this is not what we want to use.

VictorForouhar commented 5 months ago

Automated testing reveals that there are still haloes beyond the cosmological box. First image are without the changes in this branch, whereas the second one are with the current changes. Things improve but still fail image image

VictorForouhar commented 4 months ago

Latest commit fixes most issues. Comparison between the number of out-of-the-box subhaloes in original version vs updated. image

No subhaloes are found beyond the box in any of group catalogues image

VictorForouhar commented 4 months ago

Central bound mass fraction; seems like there are some differences in the total number of centrals, but unsure of whether it is caused by these changes or something else. The original test I am comparing against was done using a two-week old version, so I will re-run with the latest master branch. image

VictorForouhar commented 4 months ago

Same for satellite bound masses. Again, re-running test, but leaving for completeness image

VictorForouhar commented 4 months ago

Updated test, this time using the latest master branch. Differences in numbers of structure much smaller, but a few cases remain. Do we expect the code to be reproducible? image image

VictorForouhar commented 4 months ago

Images of the DM particles bound to the central subhaloes of the 16 most massive FOFs groups (i.e. ordered in ascending host halo id from left to right and top to bottom). First image is the master version. Seems like HostHaloId == 7 changes the most out of this set. The new version has a satellite subhalo classified as the central, and the Nbound is lower by 89 particles (0.2% of total bound ones). It is located at 4.058828, 21.43622 , 14.161549 Mpc, far away from boundaries.

image image

VictorForouhar commented 4 months ago

Additional tests; what is the typical ratio between the bound mass of centrals, after they have been matched? Typically 1 as per the distribution. There is however a central with a large mass ratio (~3.1), easily seen in the second plot. image image

VictorForouhar commented 4 months ago

Dark matter images of the sub in question (old bound vs new bound vs all). Doesn't seem like anything too crazy is going on, and it is still able to distinguish the satellite. NboundType differs between 22, 657, 0, 0, 3, 1 (old) and 112, 2003, 0, 0, 6, 1 (new) image

VictorForouhar commented 4 months ago

Matched Tracks cross versions, and we see that one of them is lost in the new version, and the other one isn't. I show their bound mass evolution below.

image image

They are never close to box boundaries. Note that these two objects are involved in a massive merger (bound mass ratio ~ 0.86), and the one identified as a central in the new version is the more massive of the two (1.44e+10 Msun vs 1.25e+10 Msun). This mass hierarchy is also present in the old version (1.52e+10 Msun vs 1.25e+10 Msun)

VictorForouhar commented 4 months ago

In fact, the one that disappears in the new version is because HBT+ forced it via the merging criterion. The question is hence why did it work in this case and not in the other?

robjmcgibbon commented 4 months ago

Most of the comments above are related to the particles being sampled during the unbinding. However, I ran this branch and the master branch using all particles for unbinding (MaxSampleSizeOfPotentialEstimate 0). There are still slightly different numbers of halos in the output, which Victor identified as being hostless halos. Until we determine the cause of these differences I don't think we should merge these changes.

VictorForouhar commented 3 months ago

Solved in PR #21