pelahi / VELOCIraptor-STF

Galaxy/(sub)Halo finder for N-body simulations
MIT License
19 stars 26 forks source link

Problem with spherical overdensity masses at z > 0 #30

Closed jchelly closed 5 years ago

jchelly commented 5 years ago

I've been doing some comparisons between Velociraptor in 3D FoF mode and the EAGLE FoF implementation and I think I might have found a bug.

At high redshift the EAGLE m200crit and EAGLE m200mean mass functions (both computed by Gadget) are very similar to each other. I think this is expected because the mean density is close to the critical density at high redshift. But Velociraptor exhibits the opposite behaviour - the m200mean mass function diverges from the m200crit mass function as z increases.

I think the problem might be with the calculation of the mean density. It's defined as

    opt.rhobg=3.*Hubble*Hubble/(8.0*M_PI*opt.G)*opt.Omega_m;

I think Omega_m here is the density parameter at z=0 because it gets read from the Omega0 header entry in the case if Gadget snapshots. Shouldn't it be the density parameter at the redshift of the snapshot?

pelahi commented 5 years ago

Thanks for spotting this John. I'll fix this tomorrow unless you want to push the correction yourself.

jchelly commented 5 years ago

I've been taking a quick look at fixing this. We'll need to correct the calculation of rhobg in the read routines and in the swift interface. The calculation of m200val and m500val in substructureproperties.cxx probably needs to be changed to use the same omega_m as rhobg too. I'm not sure which omega_m the BN98 virial overdensity calculation should be using.

pelahi commented 5 years ago

I think I've fixed the issue in the expandedproperties branch but need to test it. Will do later today.

jchelly commented 5 years ago

I just had a go at running this branch on the EAGLE 25 box and there's still something wrong - in fact now the m200m mass function at z=0 is very different from what the EAGLE code produces and both m200c and m200m look wrong at high redshift.

Looking at the latest commit in the expandedproperties branch, I think the problem is that the CalcBackgroundDensity function you modified is never called. Instead it's using the incorrect opt.rhobg calculation which is duplicated across all the read routines and the swift interface.

jchelly commented 5 years ago

Oh, sorry, I see that you already removed the duplicated calculations in the expandedproperties branch. I was looking at master. Maybe we just need to add a call to CalcBackgroundDensity.

pelahi commented 5 years ago

I think I'll also have to update the swift interface to make sure it is generalised but need to double check what Omega_i values are stored. I've pushed an update of rhobg. Can you check to see if densities and overdensity masses at high redshift are correct?

pelahi commented 5 years ago

Bug fixed. Masses match EAGLE results (DM only). fof_mass