next-exp / nexus

Geant4 simulation framework of the NEXT Collaboration
5 stars 55 forks source link

Fix for Geant4 1.11 #202

Closed atrettin closed 1 year ago

atrettin commented 1 year ago

This PR adds include lines that are necessary to define the G4double type, as necessary as of Geant4 version >11.0.

Also adds .vscode to gitignore to ignore VSCode configuration.

Now with dev branch name on my fork.

kvjmistry commented 1 year ago

Ok, if I add the header only to "BaseDriftField.h" it compiles fine using the cmake. But for using scons it breaks on the linking, something relating to the visualiser in geant4. Did you compile with scons too?

Undefined symbols for architecture arm64:
  "G4ToolsSGOffscreen::G4ToolsSGOffscreen()", referenced from:
      G4VisExecutive::RegisterGraphicsSystems() in nexus.o
ld: symbol(s) not found for architecture arm64
atrettin commented 1 year ago

Ok, if I add the header only to "BaseDriftField.h" it compiles fine using the cmake. But for using scons it breaks on the linking, something relating to the visualiser in geant4. Did you compile with scons too?

Undefined symbols for architecture arm64:
  "G4ToolsSGOffscreen::G4ToolsSGOffscreen()", referenced from:
      G4VisExecutive::RegisterGraphicsSystems() in nexus.o
ld: symbol(s) not found for architecture arm64

That is odd, because the error also happens in RadiusDependenDriftField (and given that both use the Geant4double type, it would be weird if the import was only required for one of them).

I did compile with some sort of visualization, but I'm not sure if it was "scons" specifically.

In the earlier PR, the workflow also ran without problems, so it should compile fine?

kvjmistry commented 1 year ago

Ah thats because the RadiusDependantDriftField.h has this line: #include "BaseDriftField.h" So you only need to add it to BaseDriftField.h. Are you saying it only compiles if you add it to both?

atrettin commented 1 year ago

Ah thats because the RadiusDependantDriftField.h has this line: #include "BaseDriftField.h" So you only need to add it to BaseDriftField.h. Are you saying it only compiles if you add it to both?

Ah, sorry, that makes sense of course. I didn't see that. Of course, then it is only required in one of them.

kvjmistry commented 1 year ago

For the compilation, can you try using the command scons in the main directory and see if it compiles? You may need to pass it some extra vars too like this

scons GEANT4_BINDIR=$G4INSTALL/bin GSL_BINDIR=$GSL_PATH/bin HDF5_DIR=$HDF5_PATH -j4
atrettin commented 1 year ago

For the compilation, can you try using the command scons in the main directory and see if it compiles? You may need to pass it some extra vars too like this

scons GEANT4_BINDIR=$G4INSTALL/bin GSL_BINDIR=$GSL_PATH/bin HDF5_DIR=$HDF5_PATH -j4

I will, tomorrow! :)

atrettin commented 1 year ago

It works for me, in both cases where I add or remove the line in RadiusDependentDriftField. I installed Geant4 with the settings: {"GEANT4_USE_OPENGL_X11": "ON", "GEANT4_INSTALL_DATA": "ON", "GEANT4_USE_QT": "ON"}

atrettin commented 1 year ago

But if we have these two build methods, it would be a good idea to add both of them to the GitHub workflow (but I think that goes beyond the scope of this PR).

atrettin commented 1 year ago

Speaking of which, should we bump the Geant4 version in the workflow in docker/geant4/Dockerfile?

paolafer commented 1 year ago

Speaking of which, should we bump the Geant4 version in the workflow in docker/geant4/Dockerfile?

Before changing the recommended G4 version, we usually do a validation, to check that the relevant physics stays the same across versions. If you're interested in doing this, we can talk offline.

paolafer commented 1 year ago

But if we have these two build methods, it would be a good idea to add both of them to the GitHub workflow (but I think that goes beyond the scope of this PR).

Yes, we can do it in a different PR.