Skip to content
Snippets Groups Projects

22.0-initialiseVariables-TrkTrack

Merged Shaun Roe requested to merge sroe/athena:22.0-initialiseVariables-TrkTrack into master

initialise the pointer members at declaration, and protect against a nullptr for the m_trackStateVector in one instance.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • This merge request affects 1 package:

    • Tracking/TrkEvent/TrkTrack

    Adding @amorley as watcher

  • This is fine, but do we really want to allow Trk::Track without these ? i.e I assume they always need to have them no is which case we can check even earlier. Or we really have completely empty tracks?

    Edited by Christos Anastopoulos
  • Author Developer

    Hi Christos, thanks for taking a look! I came across this in the context of unit testing collections of Trk::Tracks. In this context, it is useful to be able to create a collection of default constructed (empty) tracks for insertion to a container and test the manipulations on that container. Given that the default constructor is not deleted, subsequent manipulations should either not fail or at least fail gracefully.

  • Ah Ok, the way I uderstood it is that you encountered empty tracks in production, which seemed a bit scary..

  • The default ctor should be used only in pool. And the point is in production prb you want to fail hard. As excluding the POOL, the way I understood the comments I read in the class (pre-existing) is that there are some invariants which have to be there.

    I would have actually deleted the default ctor bar the comment

    Track (); //!<needed by POOL. DO NOT USE YOURSELF!   

    because in some sense does not fit with anything else.

    In this sense in production you "graceful" for the user would be a hard failure. i.e we should not have tracks violating

    0041      * - A pointer to a const Trk::FitQuality  - the fit quality of a track
    0042      * - A pointer to a DataVector of const Trk::TrackStateOnSurface

    as minimum.

    This is what I find confusing. Of course we can make the default ctor protected since the cnv are friends. Which was going to be my suggestion.

    But then if this is for unit tests is another bit of story.

    Obviously the pool only ctor does not fit with the invariants of the class. So is an oddity, but not sure I want to add protection for this oddity ... or just make it so as to be usable only by the friend pool converters ..

    E.g bar the POOL (which is special) no-one is supposed to create Tracks with this ctor.

    Therefore wondering if we want to enforce this more. Or if it can happen, if being "silent" about it is the way to go ?

    Edited by Christos Anastopoulos
  • CI Result SUCCESS (hash 6614ad4d)

    Athena AthSimulation AthGeneration AnalysisBase
    externals
    cmake
    make
    required tests
    optional tests

    Full details available on this CI monitor view
    Athena: number of compilation errors 0, warnings 0
    AthSimulation: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    AnalysisBase: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21940]

  • Shaun Roe mentioned in merge request !37348 (merged)

    mentioned in merge request !37348 (merged)

  • merged

  • Edward Moyse mentioned in commit dc54eafe

    mentioned in commit dc54eafe

  • mentioned in merge request !37359 (merged)

Please register or sign in to reply
Loading