pelahi / VELOCIraptor-STF

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

Incorrect values in extra output fields #78

Closed jchelly closed 4 years ago

jchelly commented 4 years ago

I'm trying to run Velociraptor on a set of EAGLE 25Mpc box simulations run using Swift. These contain black holes and I'd like to get the maximum black hole subgrid mass in each group. The corresponding dataset in the snapshot is PartType5/SubgridMasses.

I'm running commit fcfcdbdead1662cc584f5f72c0f1bc00ab2d0ace from master with 28 threads and just one MPI rank. My velociraptor cmake configuration is

cmake .. \
    -DCMAKE_BUILD_TYPE=Release \
    -DVR_USE_HYDRO=ON \
    -DCMAKE_C_COMPILER=icc \
    -DCMAKE_CXX_COMPILER=icpc

and I run it with

snapnum=0004
export OMP_NUM_THREADS=28
mpirun ./VELOCIraptor-STF/build/stf \
    -i test_data/eagle_${snapnum} \
    -I 2 \
    -s 1 \
    -C test.cfg \
    -o test_data/stf_${snapnum} \
    -Z 1

My configuration file is vrconfig_3dfof_subhalos_SO_hydro.cfg from the Swift repository but with the following added

BH_internal_property_names=SubgridMasses,SubgridMasses,SubgridMasses,SubgridMasses,
BH_internal_property_input_output_unit_conversion_factors=1.0e10,1.0e10,1.0e10,1.0e10,
BH_internal_property_calculation_type=max,min,average,aperture_total,
BH_internal_property_output_units=solar_mass,solar_mass,solar_mass,solar_mass

If I run like this then I think I should be able to read the maximum black hole mass from the dataset SubgridMasses_max_solar_mass_bh in the .properties file. However, I'm finding that the values don't match what I get if I use the catalog_properties/groups files to determine which particles are in which groups and find the maximum black hole mass myself. The python script I'm using to do this exactly reproduces the m_bh dataset in the velociraptor output if I apply it to the DynamicalMasses dataset in the swift snapshot.

I also did a test where I added a dataset called 'One' to the snapshot where all particles have the value 1.0 and I added these parameters in the .cfg file:

BH_internal_property_names=One,One,One,One,
BH_internal_property_input_output_unit_conversion_factors=1.0,1.0,1.0,1.0,
BH_internal_property_calculation_type=max,min,average,aperture_total,
BH_internal_property_output_units=unitless,unitless,unitless,unitless

The min and max values of this dataset should always be 1, but in the Velociraptor output I get a range of values around 0.2 to 0.3.

pelahi commented 4 years ago

Hi John, I also tried something similar with setting an extra field to 1 and calculating the min and max but I get only ones. I will try adding extra functions calls too so as to see if there is an issue with min,max,average,aperture_total

jchelly commented 4 years ago

Are the trailing commas required in these parameters? I just noticed I omitted one in my run and it doesn't seem to have written out the last quantity on the list.

jchelly commented 4 years ago

If I just ask for min and max I get ones as expected too.

jchelly commented 4 years ago

It turns out I can only reproduce this problem if I also have some gas properties in the .cfg file too. This combination fails for me:

Gas_internal_property_names=ElementMassFractions,ElementMassFractions,ElementMassFractions,ElementMassFractions,ElementMassFractions,
Gas_internal_property_input_output_unit_conversion_factors=1.0,1.0,1.0,1.0,1.0,
Gas_internal_property_calculation_type=logaverage,logstd,min,max,aperture_average,
Gas_internal_property_index_in_file=1,1,1,1,1,
Gas_internal_property_output_units=unitless,unitless,unitless,unitless,unitless,

BH_internal_property_names=One,One,One,One,
BH_internal_property_input_output_unit_conversion_factors=1.0,1.0,1.0,1.0,
BH_internal_property_calculation_type=max,min,average,aperture_total,
BH_internal_property_output_units=unitless,unitless,unitless,unitless

I'll see if I can narrow it down any further.

pelahi commented 4 years ago

Okay, I'll try both loading gas and bh properties. Not certain what's special about the combination but will look into it.

jchelly commented 4 years ago

If I go back to using real snapshot data and only ask for the max/min/average/aperture_total black hole subgrid mass then the output exactly agrees with my own calculation using the snapshot and the catalog_groups/particles files. So I think it's working correctly in that case.

I thought it might be something to do with reading multidimensional arrays like ElementMassFractions but it still fails if I output a 1D gas property instead, so that's not it.

pelahi commented 4 years ago

I can confirm that doing both some gas properties and some black hole properties causes calculations for both gas and bh to not work. Why, I am still digging into

pelahi commented 4 years ago

Hi @jchelly I think I have fixed the io bug in development, last two commits, specifically 3d7ae009063022b1ecff79458950898778139a17 3170aaf440958d775f74d8a3cb3e1e734c29f311 I have tested it on a small eagle run but would be good to confirm.

pelahi commented 4 years ago

HI @jchelly , I inadvertently introduced a bug in the second commit. It is now fixed in a1c920ed760473371c27a0409ab2c1a0e34e87e4 Can you verify?

pelahi commented 4 years ago

Issue resolved.