search

star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

ci: Enable tests for ROOT5 GCC11 environment #370

Closed plexoos closed 1 year ago

plexoos commented 1 year ago

This PR branch is based on and includes changes from currently unmerged #366

plexoos commented 1 year ago

Have you considered the macros that don't use BFC?

Sure, let's consider other macros. Do you have anything specific in mind?

genevb commented 1 year ago

On Jun 29, 2022, at 5:08 PM, Dmitri Smirnov @.**@.>> wrote:

Sure, let's consider other macros. Do you have anything specific in mind?

grep StEventUtilities $STAR/StRoot/macros/.C $STAR/StRoot/macros//.C /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/Load.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/Load.C:void Load(Char_t loadList="St_base,St_Tables,StChain,StDetectorDbMaker,StBichsel,StEvent,StTpcDb,StUtilities,StDbLib,StDbBroker,St_db_Maker,StTriggerDataMaker,StEventUtilities,StBFChain"){ /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/loadMuDst.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/loadMuDst.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/makePicoDst.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/makePicoDst.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/RedoSpaceCharge.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/RedoSpaceCharge.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/StMuDstMakerYear1.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/StMuDstMakerYear1.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.C:// Load StEventUtilities /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/find_vertex.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/find_vertex.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/Draw3DDoc.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/Draw3DDoc.C: doc.SetSourceDir(".:src:inc:StRoot:StRoot/StarRoot:StRoot/StEventUtilitie"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/GeomBrowse.Chttp://rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/GeomBrowse.C: gSystem->Load("StEventUtilities");

genevb commented 1 year ago

Gotta love github's automatic formatting. Trying again from the gui:

grep StEventUtilities $STAR/StRoot/macros/.C $STAR/StRoot/macros//.C /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/Load.C:void Load(Char_t loadList="St_base,St_Tables,StChain,StDetectorDbMaker,StBichsel,StEvent,StTpcDb,StUtilities,StDbLib,StDbBroker,St_db_Maker,StTriggerDataMaker,StEventUtilities,StBFChain"){ /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/loadMuDst.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/makePicoDst.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/RedoSpaceCharge.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/StMuDstMakerYear1.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/doEvents.C:// Load StEventUtilities /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/analysis/find_vertex.C: gSystem->Load("StEventUtilities"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/Draw3DDoc.C: doc.SetSourceDir(".:src:inc:StRoot:StRoot/StarRoot:StRoot/StEventUtilitie"); /afs/rhic.bnl.gov/star/packages/SL21a/StRoot/macros/graphics/GeomBrowse.C: gSystem->Load("StEventUtilities");

plexoos commented 1 year ago

Have you considered all of them when you removed the dependency on Stu?

genevb commented 1 year ago

I considered them when I added the dependency in BFC, and that drove me to avoid adding the dependency if it wasn't necessary. Thus I removed it shortly after adding it.

-Gene

On Jun 29, 2022, at 5:37 PM, Dmitri Smirnov @.**@.>> wrote:

Have you considered all of them when you removed the dependency on Stu?

plexoos commented 1 year ago

Did you run all of these macros with default parameters? And how did you define the criteria to pass/fail the tests?

genevb commented 1 year ago

Nope.

On Jun 29, 2022, at 9:07 PM, Dmitri Smirnov @.***> wrote: Did you run these macros with default parameters?

plexoos commented 1 year ago

Would you then care to share how you run the tests?

genevb commented 1 year ago

On Jun 29, 2022, at 10:48 PM, Dmitri Smirnov @.***> wrote: Would you then care to share how you run the tests?

I did not say I ran tests. But I think you've determined that StTpcDb.so needs to be loaded before StEventUtilities.so. I expect that a consequence of this is that all macros which load StEventUtilities.so must load StTpcDb.so beforehand. If those macros are not relevant for, not impacted by this PR, then you should just ignore my question about considering these macros.

-Gene

plexoos commented 1 year ago

I see the following files modified in 79e3801453d69237491e49183f62db22aa024b69 "Remove StTpcDb dependence for StEventUtilities"

 StRoot/StBFChain/BigFullChain.h                         |  2 +-
 StRoot/StEventUtilities/StEbyET0.cxx                    | 25 +++++++++++++++----------
 StRoot/StMuDSTMaker/COMMON/macros/loadSharedLibraries.C |  1 -
 StRoot/macros/analysis/doEvents.C                       |  7 +++++--
 StRoot/macros/analysis/find_vertex.C                    |  6 ++++--
 StRoot/macros/graphics/GeomBrowse.C                     |  3 +--
 StRoot/macros/loadMuDst.C                               |  1 -
 7 files changed, 26 insertions(+), 19 deletions(-)

UPDATE. The error from find_vertex.C (see test job log file https://github.com/star-bnl/star-sw/runs/7171131802)

  env:
    STARENV: root5-gcc485
 *** Start at Date : Sun Jul  3 20:13:49 2022
Processing StRoot/macros/analysis/find_vertex.C("/star/rcf/test/daq/2020/351/st_physics_20351078_raw_5000001.event.root")...
Error: Syntax error StRoot/macros/analysis/find_vertex.C:31:
Error: Missing one of ',;' expected at or after line 31.
Error: Unexpected end of file (G__fignorestream():3) StRoot/macros/analysis/find_vertex.C:261:
*** Interpreter error recovered ***
This is the end of STAR ROOT -- Goodbye
plexoos commented 1 year ago

In my previous comment I forgot to mention that running root4star -l -q 'StRoot/macros/graphics/GeomBrowse.C("y2022a") also gives me an error:

Error in <TUnixSystem::DynamicPathName>: St_geom_Maker[.so | .dll | .dylib | .sl | .dl | .a] does not exist in ...

St_geom_Maker does not exist since 2019: 173d6b42a5 - (3 years, 3 months ago) Moved to the attic - Jerome Lauret

doEvents.C can be used as a test perhaps. find_vertex.C and GeomBrowse.C do not seem to be maintained. What's the point of changing them?

So, Gene, I am not sure where we want to go from here. Revert the entire https://github.com/star-bnl/star-sw/commit/79e3801453d69237491e49183f62db22aa024b69 "Remove StTpcDb dependence for StEventUtilities" ?

genevb commented 1 year ago

If a macro requires retired code, I would consider the macro retired and remove it. -Gene

klendathu2k commented 1 year ago

Thanks!

The context is that AgML produces both C++ and FORtran sources from the xml inputs. The token "abs" is a FORtran keyword, and a C/C++ function. So both the FORtran and C++ codes need to compile.

In principle I could have added these definitions in the C++ geometry implementation files... the AbcdGeo.cxx files produced by the agml parser. I decided to collect these "hacks" inside of a single header instead, so that they wouldn't be forgotten. AgMath should not appear in any codes outside of the geometry area.

Cheers, Jason

On 2022-06-24 16:19, Dmitri Smirnov wrote:

@plexoos commented on this pull request.

In StarVMC/StarAgmlLib/AgMath.h [1]:

@@ -54,7 +53,6 @@ Int_t nint(Float_t x); // Fortran abs is fabs // //#define abs(x) TMath::Abs(x) -inline double abs (double x) {return fabs(x);}

In general, I would oppose such change in a header file... I don't understand why can't you call std::abs directly where it is supposed to be called. Perhaps I am missing the real context.. but here you go 4eeac85 [2]

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [3]. You are receiving this because your review was requested.Message ID: @.***>

Links:

[1] https://github.com/star-bnl/star-sw/pull/370#discussion_r906379879 [2] https://github.com/star-bnl/star-sw/commit/4eeac85f0fed17deca83434200159cd97e6d330c [3] https://github.com/notifications/unsubscribe-auth/ANL4LVHEDLNEJ5YL2WUHE43VQYJ4XANCNFSM5ZVAJG7A