Skip to content
Snippets Groups Projects

TileCal: creating crack scintillators in separate top-level volumes

Merged TileCal: creating crack scintillators in separate top-level volumes
solodkov/athena:tilegeo-for-24.0 into main
1 unresolved thread
Merged Sanya Solodkov requested to merge solodkov/athena:tilegeo-for-24.0 into main
1 unresolved thread

Merge request reports

Pipeline #6931159 passed

Stage: external

Pipeline passed for f222cc12 on solodkov:tilegeo-for-24.0

Approval is optional

Merged by Frank WinklmeierFrank Winklmeier 1 year ago (Feb 21, 2024 10:06am UTC)

Merge details

  • Changes merged into main with d4e2b6b5 (commits were squashed).
  • Did not delete the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Lucy Lewitt
  • Lucy Lewitt
  • Lucy Lewitt
  • Lucy Lewitt
    Lucy Lewitt @llewitt ·
    Developer

    Couple of points to address. This will need to go to L2 anyway as it's quite complicated. L1

  • Lucy Lewitt added review-user-action-required label and removed review-pending-level-1 label

    added review-user-action-required label and removed review-pending-level-1 label

  • Sanya Solodkov added 1 commit

    added 1 commit

    Compare with previous version

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    This merge request affects 2 packages:

    • TileCalorimeter/TileG4/TileGeoG4SD
    • TileCalorimeter/TileGeoModel

    Affected files list will not be printed in this case

    Adding @pavol ,@solodkov ,@harkusha ,@iouri as watchers

  • ATLAS Robot added review-pending-level-1 label and removed review-user-action-required label

    added review-pending-level-1 label and removed review-user-action-required label

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    :warning: WARNING: big files (>100K) are found in the changeset

    :pencil: 120K in file TileCalorimeter/TileG4/TileGeoG4SD/src/TileGeoG4SDCalc.cc

    :pencil: 164K in file TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx

  • Sanya Solodkov added 1 commit

    added 1 commit

    • f222cc12 - removing executable permission from all *.h and *.cxx files

    Compare with previous version

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    This merge request affects 2 packages:

    • TileCalorimeter/TileG4/TileGeoG4SD
    • TileCalorimeter/TileGeoModel

    Affected files list will not be printed in this case

    Adding @harkusha ,@solodkov ,@iouri ,@pavol as watchers

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    :warning: WARNING: big files (>100K) are found in the changeset

    :pencil: 152K in file TileCalorimeter/TileGeoModel/src/TileGeoSectionBuilder.cxx

    :pencil: 120K in file TileCalorimeter/TileG4/TileGeoG4SD/src/TileGeoG4SDCalc.cc

    :pencil: 164K in file TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    :white_check_mark: CI Result SUCCESS (hash 19cbc945)

    Athena AthSimulation
    externals :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark:
    tests :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view. Check the JIRA CI status board for known problems
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 5027]

  • ATLAS Robot
    ATLAS Robot @atlasbot ·
    Developer

    :white_check_mark: CI Result SUCCESS (hash f222cc12)

    Athena AthSimulation
    externals :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark:
    tests :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view. Check the JIRA CI status board for known problems
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 5028]

  • Yuriy Volkotrub
    Yuriy Volkotrub @yvolkotr ·
    Developer

    Changes look fine to me and the CI is good but MR given back to the developer due to the unresolved thread.

    Cheers, Yuriy (L1)

  • Yuriy Volkotrub removed review-pending-level-1 label

    removed review-pending-level-1 label

  • Yuriy Volkotrub added review-user-action-required label

    added review-user-action-required label

  • Sanya Solodkov resolved all threads

    resolved all threads

  • Sanya Solodkov added review-pending-level-1 label

    added review-pending-level-1 label

  • Sanya Solodkov removed review-user-action-required label

    removed review-user-action-required label

  • Yuriy Volkotrub
    Yuriy Volkotrub @yvolkotr ·
    Developer

    MR looks good to me, approving.

    Cheers, Yuriy (L1)

  • Yuriy Volkotrub removed review-pending-level-1 label

    removed review-pending-level-1 label

  • Yuriy Volkotrub added review-approved label

    added review-approved label

  • Frank Winklmeier merged

    merged

  • Frank Winklmeier mentioned in commit d4e2b6b5

    mentioned in commit d4e2b6b5

  • Frank Winklmeier
    Frank Winklmeier @fwinkl started a thread on the diff
    • TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx 100755 → 100644
    129 164 // Envelope creation. Building three tree tops for standard setup and only one for commissioning
    130 GeoLogVol *lvTileEnvelopeBarrel =0, *lvTileEnvelopePosEndcap =0, *lvTileEnvelopeNegEndcap =0;
    131 GeoPhysVol *pvTileEnvelopeBarrel =0, *pvTileEnvelopePosEndcap =0, *pvTileEnvelopeNegEndcap =0;
    165 GeoLogVol *lvTileEnvelopeBarrel =0, *lvTileEnvelopePosEndcap =0, *lvTileEnvelopeNegEndcap =0, *lvTileEnvelopePosCrack =0, *lvTileEnvelopeNegCrack =0;
    166 GeoPhysVol *pvTileEnvelopeBarrel =0, *pvTileEnvelopePosEndcap =0, *pvTileEnvelopeNegEndcap =0, *pvTileEnvelopePosCrack =0, *pvTileEnvelopeNegCrack =0;
    167
    168 if (crack_flag==2) {
    169 if ( m_volumeNames.size()<1 ) {
    170 (*m_log) <<MSG::WARNING << "Top-level volume names for crack scintillators are missing"<<endmsg;
    171 (*m_log) <<MSG::WARNING << "Crack scintillators will not be built"<<endmsg;
    172 crack_flag = 9;
    173 } else {
    174 GeoVolumeVec_t vols = geoGetVolumes (&*world);
    175 for (auto v : vols) {
    176 if (v.first->getLogVol()->getName() == m_volumeNames[0] )
    177 pvTileEnvelopePosCrack = (GeoPhysVol *)(v.first);
    • Frank Winklmeier
      Frank Winklmeier @fwinkl ·
      Maintainer

      This introduced a compiler warning in our dbg build:

      /build/atnight/localbuilds/nightlies/Athena/main/athena/TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx:177:34: warning: 'const' discarded from expression '<unknown>' of type 'const GeoVPhysVol*' within function 'virtual void TileAtlasFactory::create(GeoPhysVol*)'; may not be thread-safe
  177 |           pvTileEnvelopePosCrack = (GeoPhysVol *)(v.first);
      |           ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
/build/atnight/localbuilds/nightlies/Athena/main/athena/TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx:177:34: note: See <https://gitlab.cern.ch/atlas/atlasexternals/tree/master/External/CheckerGccPlugins#thread_plugin>.
/build/atnight/localbuilds/nightlies/Athena/main/athena/TileCalorimeter/TileGeoModel/src/TileAtlasFactory.cxx:179:34: warning: 'const' discarded from expression '<unknown>' of type 'const GeoVPhysVol*' within function 'virtual void TileAtlasFactory::create(GeoPhysVol*)'; may not be thread-safe
  179 |           pvTileEnvelopeNegCrack = (GeoPhysVol *)(v.first);
      |           ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
    • Please register or sign in to reply
    Please register or sign in to reply