Add a possibility to display a GDML file #28348
Conversation
|
please test |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28348/12627
|
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @ianna (Ianna Osborne) for master. It involves the following packages: Fireworks/Core @cmsbuild, @alja, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Comparison not run due to Fireworks only changes in PR (RelVals and Igprof tests were also skipped) |
cmsbuild
added
comparison-notrun
tests-approved
and removed
comparison-pending
tests-started
labels
|
@ianna , looks great. |
| s_geoManager = (TGeoManager*)file->Get(m_TGeoName.c_str()); | ||
| if (!file) { | ||
| // Try it as a GDML file | ||
| s_geoManager = TGeoManager::Import(m_fileName.c_str(), m_TGeoName.c_str()); |
There was a problem hiding this comment.
@ianna as far as I can see TGeoManager has already a protection against odd files, and in any case the final check here on the pointer not being null should be enough...
|
@alja could you please check? |
|
FWGeometry::findFile returns a nullptr if it’s not a root file.
On 6 Nov 2019, at 17:33, Fabio Cossutti <notifications@github.com<mailto:notifications@github.com>> wrote:
@fabiocos commented on this pull request.
________________________________
In Fireworks/Core/src/FWGeometryTableViewManager.cc<#28348 (comment)>:
@@ -94,12 +94,13 @@ void FWGeometryTableViewManager::setGeoManagerFromFile() {
TFile* file = FWGeometry::findFile(m_fileName.c_str());
fwLog(fwlog::kInfo) << "Geometry table file: " << m_fileName.c_str() << std::endl;
try {
- if (!file)
- throw std::runtime_error("No root file.");
-
- file->ls();
-
- s_geoManager = (TGeoManager*)file->Get(m_TGeoName.c_str());
+ if (!file) {
+ // Try it as a GDML file
+ s_geoManager = TGeoManager::Import(m_fileName.c_str(), m_TGeoName.c_str());
@ianna<https://github.com/ianna> as far as I can see TGeoManager has already a protection against odd files, and in any case the final check here on the pointer not being null should be enough...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#28348?email_source=notifications&email_token=AAKTQWVOUPW6MD3NKQXDSN3QSLWUDA5CNFSM4JJDFST2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKQX2FA#pullrequestreview-312573204>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAKTQWWE44JTOGMFX36JW5LQSLWUDANCNFSM4JJDFSTQ>.
|
|
@fabio I think Yana's change is OK. But, I would change FWGeometry::findPath() [1] to locate just a file location without creating the TFile object. This will make a consistent search for a geo files in given environment. @yana can you also make that change? https://github.com/cms-sw/cmssw/blob/master/Fireworks/Core/src/FWGeometry.cc#L26 |
|
@alja @ianna thanks for your comments, mine was not a doubt but just stating that the code should be safe also because of the TGeoManager built-in protection. @ianna there is a suggestion for an update (with the wrong address, may be you did not see it) #28348 (comment) |
|
@fabiocos and @alja - I think, searching for the GDML files in the CMSSW paths is not required since it's not a standard geometry file, but rather an import from another tool for debugging purposes only. Using |
|
@ianna , @fabiocos , @alja , as far as I know, we do not have any gdml files inside CMSSW data. Production of a new gdml for any version of CMS or a sub-detector takes few minutes. In our practice, we provide these gdml for external use, for example, to study geometry performance by geometry experts of Geant4. I am not sure, if it is required to have an official gdml files supported by CMSSW external data facility. So, for me this PR is fine. |
|
@ianna Thanks Yana for the explanation. To me PR is fine too. |
|
@alja apparently your +1 has not been seen by the bot as it is not the first character, anyway your will is clear, thanks |
|
+1 |
|
merge |
PR description:
A GDML file produced by G4 GDML writer can be viewed in cmsShow. For example:
@civanch - FYI
PR validation:
if this PR is a backport please specify the original PR:
Before submitting your pull requests, make sure you followed this checklist: