search

ICRAR / VELOCIraptor-STF

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

Compilation without HYDRO but with GAS broken #70

Closed MatthieuSchaller closed 3 years ago

MatthieuSchaller commented 3 years ago

Describe the bug Latest master. Configure with VR_USE_GAS but not VR_USE_HYDRO.

Error

In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/stf.h:8,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/buildandsortarrays.cxx:5:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/stf.h:8,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/haloproperties.cxx:7:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.cxx:5:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/logging.h:7,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/bgfield.cxx:7:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/logging.h:7,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:26:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/stf.h:8,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/fofalgo.cxx:5:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/stf.h:8,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/io.cxx:7:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~
      |   aperture_Z_gas
In file included from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/stf.h:8,
                 from /cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/gadgetio.cxx:7:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h: In member function ‘void PropData::ConverttoComove(Options&)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/allvars.h:2630:3: error: ‘aperture_M_gas_highT’ was not declared in this scope; did you mean ‘aperture_Z_gas’?
 2630 |   aperture_M_gas_highT[i]*=opt.h;
      |   ^~~~~~~~~~~~~~~~~~~~

Should be easy to fix; most likely an incorrect #ifdef choice in these files when treating the newly added quantities from #57.

MatthieuSchaller commented 3 years ago

In the same vain, if I use VR_USE_STAR but not VR_USE_HYDRO (and not VR_USE_GAS), I get:

cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/gadgetio.cxx: In function ‘void ReadGadget(Options&, std::vector<NBody::Particle>&, Int_t, NBody::Particle*&, Int_t)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/gadgetio.cxx:1155:33: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
 1155 |                 Pbuf[ibufindex].SetZmet(0);
      |                                 ^~~~~~~
make[2]: *** [src/CMakeFiles/velociraptor.dir/gadgetio.cxx.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx: In function ‘void ReadHDF(Options&, std::vector<NBody::Particle>&, Int_t, NBody::Particle*&, Int_t)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:2777:41: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
 2777 |                         Pbuf[ibufindex].SetZmet(0);
      |                                         ^~~~~~~
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:2825:41: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
 2825 |                         Pbuf[ibufindex].SetZmet(Zdoublebuff[nn]);
      |                                         ^~~~~~~
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:2825:49: error: ‘Zdoublebuff’ was not declared in this scope; did you mean ‘vdoublebuff’?
 2825 |                         Pbuf[ibufindex].SetZmet(Zdoublebuff[nn]);
      |                                                 ^~~~~~~~~~~
      |                                                 vdoublebuff
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:3058:41: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
 3058 |                         Pbuf[ibufindex].SetZmet(0);
      |                                         ^~~~~~~
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:3093:43: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
 3093 |                           Pbuf[ibufindex].SetZmet(Zdoublebuff[nn]);
      |                                           ^~~~~~~
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/hdfio.cxx:3093:51: error: ‘Zdoublebuff’ was not declared in this scope; did you mean ‘vdoublebuff’?
 3093 |                           Pbuf[ibufindex].SetZmet(Zdoublebuff[nn]);
      |                                                   ^~~~~~~~~~~
      |                                                   vdoublebuff
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/io.cxx: In function ‘void AdjustStarQuantities(Options&, std::vector<NBody::Particle>&, Int_t)’:
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/io.cxx:167:15: error: ‘class NBody::Particle’ has no member named ‘SetZmet’
  167 |             p.SetZmet(p.GetZmet()*opt.metallicityinputconversion);
      |               ^~~~~~~
/cosma7/data/dp004/jlvc76/VELOCIraptor/VELOCIraptor-STF/src/io.cxx:167:25: error: ‘class NBody::Particle’ has no member named ‘GetZmet’
  167 |             p.SetZmet(p.GetZmet()*opt.metallicityinputconversion);
MatthieuSchaller commented 3 years ago

Compiling with both VR_USE_STAR and VR_USE_GAS works.

rtobar commented 3 years ago

I fixed the first issue on the issue-70 branch. However that made another problem with only VR_USE_GAS=ON surface:

[ 44%] Building CXX object src/CMakeFiles/velociraptor.dir/substructureproperties.cxx.o
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx: In function ‘void GetFOFMass(Options&, Int_t, NBody::Particle*, Int_t, Int_t*&, Int_t*&, PropData*&, Int_t*&)’:
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3107:23: error: ‘class NBody::Particle’ has no member named ‘GetSFR’
 3107 |      auto sfr = Pval->GetSFR();
      |                       ^~~~~~
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3120:74: error: ‘class NBody::Particle’ has no member named ‘GetZmet’
 3120 |                        pdata[i].Z_mean_gas_highT_incl += massval * Pval->GetZmet();
      |                                                                          ^~~~~~~
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx: In function ‘void GetFOFMass(Options&, Int_t, Int_t*&, PropData*&)’:
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3172:50: error: ‘struct PropData’ has no member named ‘M_gas_nsf’; did you mean ‘M_gas_incl’?
 3172 |      pdata[hostindex].M_gas_nsf_incl += pdata[i].M_gas_nsf;
      |                                                  ^~~~~~~~~
      |                                                  M_gas_incl
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3173:49: error: ‘struct PropData’ has no member named ‘M_gas_sf’; did you mean ‘M_gas’?
 3173 |      pdata[hostindex].M_gas_sf_incl += pdata[i].M_gas_sf;
      |                                                 ^~~~~~~~
      |                                                 M_gas
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3190:49: error: ‘struct PropData’ has no member named ‘M_gas_nsf’; did you mean ‘M_gas_incl’?
 3190 |             pdata[i].M_gas_nsf_incl += pdata[i].M_gas_nsf;
      |                                                 ^~~~~~~~~
      |                                                 M_gas_incl
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3191:48: error: ‘struct PropData’ has no member named ‘M_gas_sf’; did you mean ‘M_gas’?
 3191 |             pdata[i].M_gas_sf_incl += pdata[i].M_gas_sf;
      |                                                ^~~~~~~~
      |                                                M_gas
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx: In function ‘void GetSOMasses(Options&, Int_t, NBody::Particle*, Int_t, Int_t*&, PropData*&)’:
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3416:46: error: ‘class NBody::Particle’ has no member named ‘GetSFR’
 3416 |                sfr[j] = Part[taggedparts[j]].GetSFR();
      |                                              ^~~~~~
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3418:47: error: ‘class NBody::Particle’ has no member named ‘GetZmet’
 3418 |                Zgas[j] = Part[taggedparts[j]].GetZmet();
      |                                               ^~~~~~~
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3484:73: error: ‘class NBody::Particle’ has no member named ‘GetSFR’
 3484 |                             sfr[offset+j] = PartDataGet[taggedparts[j]].GetSFR();
      |                                                                         ^~~~~~
/home/rtobar/scm/git/VELOCIraptor-STF/src/substructureproperties.cxx:3486:74: error: ‘class NBody::Particle’ has no member named ‘GetZmet’
 3486 |                             Zgas[offset+j] = PartDataGet[taggedparts[j]].GetZmet();
      |                                                                          ^~~~~~~
MatthieuSchaller commented 3 years ago

It seems that the code now assumes that VR_GAS means that the particles carry SFR and Z. That wasn't the case ~1 year ago. That's a fine change to make but it may have broken some people's test cases. In particular people running just so-called adiabatic gravity+gas models may not be happy.

That same assumption is done if I run the code with

Input_includes_gas_particle=1
Input_includes_star_particle=0

in the config file. The code will search for a SFR and metallicity field in the snapshot, which may not exist.

rtobar commented 3 years ago

I tried some more again today, trying to find out when exactly these builds were broken. Unfortunately the commit history didn't lend itself for an easy git bisect, as there are many commits that break the build, which is then fixed in a subsequent commit (so there's no a stable basis to use for comparison).

Regardless, I still found that in 62fbb10 (1y9m ago) the GAS-only build works, but the STAR-only build doesn't, with very similar errors to the ones reported above, which indicates this build combination has been broken for a long time. In the case of the GAS-only compilation errors, they all seemed related to the new hot properties added in #57, and indeed the master branch before that particular merge (i.e., 08595f8) compiles fine.

In summary:

MatthieuSchaller commented 3 years ago

STAR-only is quite a strange choice to be fair so I am not surprised it has been broken for a long time. I would not expect many people to ever use this. However, GAS-only is meaningful and I think was used by some of the ICRAR student projects as well. Maybe fixing just this is the most important bit and the STAR-only aspect can be left out.

edoaltamura commented 3 years ago

Hi all, I wanted to add something hopefully related to this. I am trying to make catalogues for adiabatic runs (DM and gas only) and compiling with

cmake . \
    -DVR_USE_HYDRO=ON \
    -DVR_USE_GAS=ON \
    -DVR_USE_STAR=OFF \
    -DVR_USE_BH=OFF \
    -DVR_USE_SWIFT_INTERFACE=OFF \
    -DCMAKE_CXX_FLAGS="-fPIC" \
    -DCMAKE_BUILD_TYPE=Release \
    -DVR_ZOOM_SIM=ON \
    -DVR_MPI=OFF

The compilation is successful, however, at runtime I came across i/o errors related to the SFR. The SFR field in SWIFT is normally attached to gas particle types, but it's not dumped when the simulation is run in adiabatic mode. Setting VR_USE_HYDRO=ON and VR_USE_STAR=OFF apparently avoids the compilation errors, but makes the code look for a SFR field in PartType0 which may not be there if stars aren't included in the model. @MatthieuSchaller suggested deleting all mentions to SFR in the parameter file (having the appropriate PartTypes switched off), but the code still needs info related to the SFR. Is there anything obvious I'm missing or is this behavior actually related to what Matthieu pointed out above? Thanks in advance for helping!

rtobar commented 3 years ago

@MatthieuSchaller @edoaltamura @JBorrow yesterday I had a chance to have a look at this with @cdplagos, and were able to produce a fix for this issue. Would you mind testing the contents of the issue-70 branch? I've also added a new job to the Travis CI build matrix to ensure gas-only builds are not broken again.

MatthieuSchaller commented 3 years ago

Thanks! Will do.

MatthieuSchaller commented 3 years ago

I can confirm it compiles and runs smoothly!

rtobar commented 3 years ago

Thanks @MatthieuSchaller for the good news :). I'll wait for further confirmation from @JBorrow to double-check that his use case reported in #91 has also been solved by these fixes; after that I'll merge to the master branch.

JBorrow commented 3 years ago

Yes, it now works (although my test case dies with the vr::non_positive_density error in OMP only mode). Thanks for the fix!

rtobar commented 3 years ago

Fix merged to the master branch, closing this now. Thanks for the patience!