Aperture quantities for extra quantity broken for `aperture_total` about velociraptor-stf HOT 18 CLOSED

Hi @MatthieuSchaller , I'll look into this later tonight.

from velociraptor-stf.

Actually, it looks like only the first 8 calculation options are actually implemented. There is also some strange overflow + modulo combinations in substructureproperties.cxx when setting the functions in lines 6335 and following.

from velociraptor-stf.

To be more specific, here is an example:

• Input file: /cosma7/data/dp004/jlvc76/COLIBRE/cold_gas/colibre_0023.hdf5
• Config file: /cosma7/data/dp004/jlvc76/COLIBRE/cold_gas/vrconfig_3dfof_subhalos_SO_hydro.cfg
• Version: 5eb6351
• Compilation line: cmake ../ -DVR_USE_HYDRO=ON -DCMAKE_BUILD_TYPE=Debug -DVR_OPENMP=OFF
• Runtime command: stf -I 2 -i colibre_0023 -o haloes -C vrconfig_3dfof_subhalos_SO_hydro.cfg

To get a version that works, comment out line 179 of the configuration and comment in line 180.

from velociraptor-stf.

One additional related thing. If you add aperture_total to a list of things that contain other operations then the code does not crash but returns an array of 0s in the catalogue.

For instance

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,

does not crash but produces and array of 0 for the last quantity whilst the first three are fine.

from velociraptor-stf.

Hi @rtobar @MatthieuSchaller
I think I've fixed all the bugs for apertures. The seg fault when running with only apertures loaded was a silly bug that is fixed. I've tested the branch with colibre data, loading in densities for -DVR_MPI=OFF and mpi on. Seems to work. I will do some further testing but the branch is now available. @rtobar you could cherry pick the commits to a local ICRAR branch.
https://github.com/pelahi/VELOCIraptor-STF/tree/bugfix/load_extra_propeties_for_apertures

from velociraptor-stf.

Nice. Thanks very much for the prompt response!

I am happy to test it as well. Note that beyond the segfault (which was more of a symptom I think) the properties arrays ended up filled with zeros.

from velociraptor-stf.

The zeros should also be fixed. Based on the colibre snapshot, the values also seemed reasonable too.

from velociraptor-stf.

I'll try to cherry-pick the changes into a new issue-72 branch (gave it a quick crack, but needs some manual merging, so I'll have to look a bit closer into it). After that we can test the changes in cosma against the original inputs.

from velociraptor-stf.

@MatthieuSchaller: @pelahi's changes have been pushed now to the issue-72 branch, please try them out at your earliest convenience.

from velociraptor-stf.

Just tried that branch on the example above. It dies with:

[0000] [   1.668] [ info] hdfio.cxx:2447 Reading file 0
[0000] [   1.668] [debug] hdfio.cxx:1244 Opening Dataset PartType0/Coordinates
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType1/Coordinates
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType4/Coordinates
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType5/Coordinates
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType0/Velocities
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType1/Velocities
[0000] [   1.669] [debug] hdfio.cxx:1244 Opening Dataset PartType4/Velocities
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType5/Velocities
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType0/ParticleIDs
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType1/ParticleIDs
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType4/ParticleIDs
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType5/ParticleIDs
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType0/Masses
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType1/Masses
[0000] [   1.670] [debug] hdfio.cxx:1244 Opening Dataset PartType4/Masses
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType5/DynamicalMasses
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType0/InternalEnergies
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType0/StarFormationRates
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType0/MetalMassFractions
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType4/MetalMassFractions
[0000] [   1.671] [debug] hdfio.cxx:1244 Opening Dataset PartType4/BirthScaleFactors
[0000] [   1.672] [debug] hdfio.cxx:1244 Opening Dataset PartType4/BirthDensities
[0000] [   1.672] [debug] hdfio.cxx:1244 Opening Dataset PartType5/SubgridMasses
HDF5-DIAG: Error detected in HDF5 (1.10.6) MPI-process 0:
  #000: H5Dio.c line 185 in H5Dread(): could not get a validated dataspace from file_space_id
    major: Invalid arguments to routine
    minor: Bad value
  #001: H5S.c line 255 in H5S_get_validated_dataspace(): selection + offset not within extent
    major: Dataspace
    minor: Out of range
Error in HDF routine 
Abort(1) on node 0 (rank 0 in comm 0): application called MPI_Abort(MPI_COMM_WORLD, 1) - process 0

Note in particular that the new fields I am asking to read are not listed in Opening Dataset xxx. However, if I revert to using another kind of property calculation (say average) then it is listed there and the code runs.

from velociraptor-stf.

I think I've fixed the MPI error. There's an update to the branch

from velociraptor-stf.

@MatthieuSchaller I've just cherry-picked the latest update from @pelahi and put it in the issue-72 branch for you to test.

from velociraptor-stf.

Thanks!

The code does not crash anymore at that point. However, the fields are still not listed so I am not sure they are actually read in.

FYI, I am compiling with -DVR_USE_HYDRO=ON -DVR_OPENMP=OFF -DVR_MPI=ON.

from velociraptor-stf.

The code completed succesfully and the arrays are filled with non-zero values. I'll check them today.

from velociraptor-stf.

All good! The new properties are all computed correctly.

from velociraptor-stf.

As an additional check, this also now worked correctly when using OpenMP.

from velociraptor-stf.

Thanks a lot @pelahi for providing the fixes for this problem, and to @MatthieuSchaller for initially reporting and now verifying the fixes. After a final test round via #74 I merged the issue-72 branch into the master, so we can probably declare this issue as fixed.

from velociraptor-stf.

As always, thanks for the prompt and efficient response guys! Much appreciated.

from velociraptor-stf.