Skip to content

prep for v5 release of ldmx/dev image #1656

Closed
@tomeichlersmith

Description

@tomeichlersmith

The v5 release of the ldmx/dev image comes with a new compiler version (amongst other dependency updates) which propagates to ldmx-sw as more/different warnings. These additional warnings need to be patched in order to maintain our strict building rules and keep the CI tests passing.

Testing Set Up

just clean
just use ldmx/dev:5.0.0-rc2
mkdir build
unbuffer just configure |& tee build/configure.log
unbuffer just build 1 |& tee build/build.log
# unbuffer is from the `expect` package and is used to trick just and its child commands into using colors

build.log
configure.log

(view with less -R otherwise you will see the ANSI color codes written into the files)

To Do List

I'll keep editing this as I make progress with testing.

  • patches to ldmx-sw following compiler update #1640
  • only fetch acts and compile it if not found in image #1649
  • the HcalDigiPipelineTest is now failing 812 / 1000 of the test energies

    All of the test hits being reconstructed return a daq_pe of 0 rather than something close to the real value. The tests below 40PE are passing because 0 is within 40 of the true value. I don't think this is as apocalyptic as it seems because the PR validations all pass and those include distributions on non-zero PE hits in the HCal. I am guessing it has to do with how the test is creating the hits or how Catch2 is working with the new compiler.
    I am now unable to replicate this with the centrally-built image and a from-scratch build. Going to assume I did not fully clean the build or something when originally seeing this issue.
  • use after free warning in EcalSD, TaggerHitFilter

    This comes from the touchableHandle being a reference-counted object that is de-allocated after being accessed. I think we can avoid this issue by constructing a reference rather than a copy.
  • potential null pointer dereference using G4String copy constructor

    This originates in many user actions in Biasing in many similar places as the GetLogicalVolume dereference below. I think both can be resolved by adding explicit checks that the volumes are not nullptr.
  • potential null pointer dereference calling GetLogicalVolume in a lot of the user actions

    Resolving this issue by inserting more checks requires editing a lot of the same places that Iss1376 geant4 object pointer for comparison #1633 is touching so I'm going to wait for that PR before patching further.
  • (clang-lto) using integer absolute value function abs when argument is a float (EcalGeometry.cxx:98, MyClusterWeight.h:42, TrackDeDxMassEstimator.cxx:63, TrackerVetoProcessor.cxx:43,45,62, TrigElectronProducer.cxx:158,159)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions