Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor updates for TRestGDMLParser #202

Merged
merged 17 commits into from
May 10, 2022
Merged

Minor updates for TRestGDMLParser #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9f295b4
TRestGDMLParser.cxx - better debugging message
lobis May 9, 2022
f0ebe59
TRestGDMLParser - modernized class, normalized member names, increase…
lobis May 9, 2022
1c6fce3
TRestGDMLParser - `fOutputGdmlDirectory` now uses "/tmp/gdml" if REST…
lobis May 9, 2022
d23d41f
TRestGDMLParser - using class name in prints
lobis May 9, 2022
2bc7e4e
TRestGDMLParser - renamed `CreateGeoM` method to `CreateGeoManager`, …
lobis May 9, 2022
bc07e79
TRestGDMLParser - added <filesystem> header to source file. Check GDM…
lobis May 9, 2022
90eaa64
TRestGDMLParser - restored removed `CreateGeoM` method and added depr…
lobis May 9, 2022
d58379b
Replaced `Nevents` by `nEvents` globally (old way still works)
lobis May 9, 2022
1ffc4f5
Globally replaced `gdml_file` by `gdmlFile` (old way still works)
lobis May 9, 2022
3fa0573
TRestMetadata - fix some typos
lobis May 9, 2022
226b8e6
Merge remote-tracking branch 'origin/master' into lobis-activeVolume-43
lobis May 9, 2022
0d73a2b
Removing unnecesary stuff in Testing.cmake
juanangp May 6, 2022
172ba77
Replacing gtest_discover_tests by gtest_add_tests to avoid errors dur…
juanangp May 6, 2022
887ef17
Bug fix loadG4 macro while generating thisREST.sh
juanangp May 6, 2022
c497cb5
.gitlab-ci.yml - added test output on failure only
lobis May 10, 2022
26647a2
Not using `#define` in tests when possible
lobis May 10, 2022
ffb4790
Fix some typos, inline methods
lobis May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Build and Test:
- cmake ${CI_PROJECT_DIR}
-DTEST=ON -DREST_GARFIELD=OFF -DREST_G4=ON -DRESTLIB_DETECTOR=ON -DRESTLIB_RAW=ON -DRESTLIB_TRACK=ON
- make -j2
- ctest --verbose -O ${CI_PROJECT_DIR}/build/Testing/summary.txt
- ctest --output-on-failure -O ${CI_PROJECT_DIR}/build/Testing/summary.txt

artifacts:
name: "Testing"
Expand Down
21 changes: 6 additions & 15 deletions cmake/Testing.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ if (TEST)
add_compile_definitions(REST_TESTING_ENABLED)
endif ()

macro(ADD_TEST)
if (TEST)
message(STATUS "Adding tests at ${CMAKE_CURRENT_SOURCE_DIR}")
add_subdirectory(test)
endif ()
endmacro()

macro(ADD_LIBRARY_TEST)
if (TEST)
message(STATUS "Adding tests at ${CMAKE_CURRENT_SOURCE_DIR}")
Expand All @@ -31,13 +24,9 @@ macro(ADD_LIBRARY_TEST)

enable_testing()

add_executable(${TESTING_EXECUTABLE})

FILE(GLOB SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/test/src/*.cxx)
target_sources(
${TESTING_EXECUTABLE} PUBLIC
${SOURCES}
)

add_executable(${TESTING_EXECUTABLE} ${SOURCES})

target_link_libraries(
${TESTING_EXECUTABLE} PUBLIC
Expand All @@ -49,7 +38,9 @@ macro(ADD_LIBRARY_TEST)

include(GoogleTest)

gtest_discover_tests(${TESTING_EXECUTABLE})

gtest_add_tests(
TARGET ${TESTING_EXECUTABLE}
SOURCES ${SOURCES}
)
endif ()
endmacro()
2 changes: 1 addition & 1 deletion cmake/thisREST.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ string(REGEX REPLACE "\n$" "" GEANT4_PATH "${GEANT4_PATH}")
set(thisGeant4 "${GEANT4_PATH}/bin/geant4.sh")

if (${REST_G4} MATCHES "ON")
set(loadG4 "\# if geant4.sh script is found we load the same Geant4 version as used in compilation\n if [[ -f \"${thisGeant4}\" && ${thisGeant4} != /usr/* ]]; then
set(loadG4 "\# if geant4.sh script is found we load the same Geant4 version as used in compilation\n if [[ -f \\\"${thisGeant4}\\\" && ${thisGeant4} != /usr/* ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a bug fix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure, it was introduced by @juanangp

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a bug fix, it was giving an error after performing cmake

source ${thisGeant4}\n fi\n")
else ()
set(loadG4 "")
Expand Down
6 changes: 3 additions & 3 deletions pipeline/pandaxiii_MC/Xe136bb0n.rml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
</TRestRun>

<TRestGeant4Metadata name="restG4 Simulation run" title="Simulation of Xe136bb0n decay from gas.">
<!-- <parameter name="Nevents" value="1000000"/>-->
<parameter name="Nevents" value="500"/>
<parameter name="gdml_file" value="geometry/main.gdml"/>
<!-- <parameter name="nEvents" value="1000000"/>-->
<parameter name="nEvents" value="500"/>
<parameter name="gdmlFile" value="geometry/main.gdml"/>
<parameter name="subEventTimeDelay" value="100us" />
<parameter name="seed" value="1" />

Expand Down
4 changes: 2 additions & 2 deletions pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

<TRestGeant4Metadata name="restG4 Simulation run" title="Simulation of Th232 decay from vessel.">

<parameter name="Nevents" value="4000000"/>
<parameter name="gdml_file" value="/home/zhoubugao/REST/geometry_bgzou/main.gdml"/>
<parameter name="nEvents" value="4000000"/>
<parameter name="gdmlFile" value="/home/zhoubugao/REST/geometry_bgzou/main.gdml"/>
<parameter name="maxTargetStepSize" value="500" units="um" />
<parameter name="subEventTimeDelay" value="100" units="us" />

Expand Down
48 changes: 25 additions & 23 deletions source/framework/core/inc/TRestGDMLParser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,51 @@

#include "TRestMetadata.h"

///////////////////////////////////////////
// we must preprocess gdml file because of a bug in TGDMLParse::Value() in ROOT6
//

class TRestGDMLParser : public TRestMetadata {
private:
TGeoManager* fGeo;
TGeoManager* fGeoManager{};

public:
TRestGDMLParser() {}
~TRestGDMLParser() {}
std::string filestr = "";
std::string path = "";
std::string outPath = REST_USER_PATH + "/gdml/";
std::string outfilename = "";
std::string gdmlVersion = "0.0";
std::map<std::string, std::string> entityVersion;
TRestGDMLParser() = default;
~TRestGDMLParser() = default;

std::string GetEntityVersion(std::string name);
std::string fFileString = "";
std::string fPath = "";
std::string fOutputGdmlDirectory = REST_USER_PATH.empty() ? "/tmp/gdml/" : REST_USER_PATH + "/gdml/";
std::string fOutputGdmlFilename = "";
std::string fGdmlVersion = "0.0";
std::map<std::string, std::string> fEntityVersionMap{};

std::string GetGDMLVersion();
std::string GetEntityVersion(const std::string& name) const;

std::string GetOutputGDMLFile();
inline std::string GetGDMLVersion() const { return fGdmlVersion; }

void Load(std::string file);
inline std::string GetOutputGDMLFile() const { return fOutputGdmlFilename; }

TGeoManager* GetGeoManager(std::string gdmlFile) {
Load(gdmlFile);
fGeo = TGeoManager::Import(GetOutputGDMLFile().c_str());
return fGeo;
void Load(const std::string& filename);

inline TGeoManager* GetGeoManager(const std::string& gdmlFilename) {
Load(gdmlFilename);
fGeoManager = TGeoManager::Import(GetOutputGDMLFile().c_str());
return fGeoManager;
}

TGeoManager* CreateGeoM();
TGeoManager* CreateGeoManager();
[[deprecated("Use CreateGeoManager() instead.")]] inline TGeoManager* CreateGeoM() {
return CreateGeoManager();
}

void InitFromConfigFile() {}
void InitFromConfigFile() override {}

void PrintContent();

void ReplaceEntity();

void ReplaceAttributeWithKeyWord(std::string keyword);
void ReplaceAttributeWithKeyWord(const std::string& keyword);

ClassDef(TRestGDMLParser, 1);
ClassDefOverride(TRestGDMLParser, 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For curiosity, when should we use DefOverride and why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use ClassDefOverride in all the inherited functions, also we should mark all the inherited virtual functions with override, e.g.:

class Parent {
virtual void myInheritedFunction();
ClassDef(Parent, 1);
}
class Children : public Parent {
void myInheritedFunction() override;
ClassDefOverride(Children, 1);
}

In general is a good practise to mark inherited virtual functions with override, otherwise is difficult to know if the function is inherited. If ClassDefOverride is not declared, root gives a lot of warnings while labeling functions as override

I think we should update the template macros to generate different processes or metadata accordingly.

};

#endif
Loading