LArSoft Coordination Meeting
Tuesday, November 6, 2018

9:00 Erica Snider - Release and Project Report

  Last week
  
    - v07_09_00 release Nov 1

      o OpFastScintillation cleanup
      o LArG4 phase 2 changes
      o LArWireCell update

    - v07_08_00_01 released Nov 2

      o ProtoDUNE production release
      o New version of ifbeam (v2_2_10)

  Status of art v3 migration
 
    - art v3 v08_00_00rc0

      o Will need feedback from uboone & protodune

   ProtoDUNE next production version in December

   Uboone freezes Nov 12, and again possibly in December

   Notes about adding new features to common code

     - Should give summary to larsoft@fnal.gov or at the coordination meeting
     - Should also comment the code using doxygen format
     - Provide Lynn Garren release notes at larsoft-team@fnal.gov 

9:09 Mike Wang - Multi-threaded LArSoft Services

  - Art v3 allows multiple events within the same SubRun to run in parallel

  - Simple services can be made thread-safe by inserting a use of
    RecursiveMutexSentry into each member function.

  - This is not always enough for more complicated services.

    For example:

      o DetectorClocksService - has per-event state
      o LArFFT - has per-event & per-wire state

  - Art v3 not quite yet ready, release candidate coming soon, so a
    standalone testing environment for LArSoft services has been developed.

  - Work will start with the LArFFT service.


9:14 Giuseppe Cerati - Plans for migrating from doubles to floats in specific
     data objects

  - Uboone will soon start a large production campaign, and will need to
    address the file staging from tape bottleneck.  To do this it must reduce
    the size of data files, and it can get a > 20% reduction by converting
    double to float in certain data products.

  - Tracks, Calorimetry, SpacePoint, MCSFitResult, and TrackFitHItInfo are
    the classes which are targeted to be reduced in size.

  - Other classes are already float-based, or not big enough to matter.

  - The approach is to change double to the root data type Double32_t to get
    smaller disk size but preserve double precision in memory and in the API.

  - Floating-point precision at DUNE scales

    o Largest coordinate value is 65m coordinate at end of an FD module
    o With float we get stable resuls down to 100um where we hit a level
      of 5% fluctuations
    o In the future may move origin to middle or store a per-track offset

  - Track and SpacePoint
    o Replace doubles with Double32_t
    o Change is in TrackingTypes.h
    o tracking::Point_t is now different from geo::Point_t (same for Vector_t)
    o SVector and SMatrix now use Double32_t

  - MCSFitResult and TrackFitHItInfo
    o double -> float
    o SVector5 and SMatrixSym55 -> Double32_t
    o Breaks API

  - Calorimetry
    o double -> float
    o fXYZ: TVector3 to Point_t<Double32_t>
    o Breaks API

  - Status
    o Nearly complete
    o Chanage are to lardataobj on branch feature/cerati_double2float_v2

  - Still to be done:
    o Examine the I/O rules
      - Do we need a rule for float -> Double32_t?
    o Fix code for breaking changes
    o Documentation
    o Track API change
      - Replace return of TVector3 with Point/Vector_t

  - Other changes

    o Updates the calib workflow require correcting the track positions for
      the SCE
      - Also need to retrieve the original positions and hit used in the track

    o Proposal:
      - Use fXYZ for SCE corrected positions
      - Add vector<size_t> to store positions of the corresponding
        TrajectoryPoints in the track
      - Hits can be retrieved either through the TrajectoryPointFlags or
        by assuming the hit association is in the same order as the
        TrajectoryPoints

  - Discussion Points

    o Double32_t choice is another ROOT-based feature, this choice makes
      it harder to move to an alternative data persistency mechanism
      like HDF5
    
    o The Double32_t type is badly named, it implies the on-disk format is
      double and it is not, which is confusing for developers.

    o Keeping the API in double has advantages since the precision is
      needed for intermediate calculations

    o Explicit conversion to float for I/O is hard to arrange because the
      on-disk format is determined by the type of the data members.

    o Mu2e comments that they are currently discussing what to demote to
      float for disk usage.

    o Everybody agrees that it is not exactly clear what actually requires
      double precision to get sufficiently accurate results.

    o Going to float instead of Double32_t will require algorithm code to
      be changed to explicitly cast data members to double before performing
      calculations.

9:55 Kyle Knoepfel - TrackMomentumCalculator changes

  - Problems found in the code

    o Incorrectly specified standard header names (use #include <> form!)
    o Should not have "using namespace" in header files
    o Global variables in header file violates the ODR unless declared extern
    o Classes should not have public data members
    o Classes with unnecessary data members
    o Classes with unused setter functions

  - Fixes are on branch: feature/knoepfel_TrackMomentumCalculator_cleanup

    o larreco
    o lariatsoft
    o ubana
    o dunetpc

  - Changes include

    o SetMinLength() and SetMaxLength() removed, set by ctor
    o many functions -> private
    o publice data members -> private
    o removed global variables and using namespace from header files
    o will be in this week's release

9:58 Herb Greenlee - Teaching Database Services to be Lazy

  - Background

    o Standard fcl files load all services

    o The service ctors access the database over the network and overload
      the server during job startup

    o Need to modify ctors to not contact the server immediately

  - Description

    o art svc -> larsoft provider -> DatabaseRetrievalAlg -> DBFolder

      - art svc

        o owns provider
        o PreProcessEvent callback calls provider function Update(timestamp)

    o provider

      - caches data from DB
      - derives from DatabaseRetrievalAlg
      - update func calls DatabaseRetrievalAlg::UpdateFolder(timestamp)

    o DatabaseRetrievalAlg

      - owns DBFolder
      - understands IOVs
      - UpdateFolder function uses DBFolder::UpdateData(timestamp)

    o DBFolder

      - Does actual query to DB server

  - Proposed changes to providers

    o Add data member fCurrentTimeStamp (cached data)

    o Add data member fEventTimeStamp (most recent event)

    o Add functions to update these values

      - DoUpdate(timestamp) for fCurrentTimeStamp
      - UpdateTimeStamp(timestamp) for fEventTimeStamp

    o Changes to existing functions and data members

      - Update(timestamp) calls DoUpdate(timestamp)
      - Make all data members modified by DoUpdate(timestamp) mutable
      - Call DoUpdate(fEventTimeStamp) from all accessors that use the
        mutable data members

  - Proposed changes to services

    o Modify PreProcessEvent callback to call provider function
      UpdateTimeStamp(timestamp) instead of Update(timestamp)

  - Further explanation

    o These are non-breaking changes for service users.

      - The only visible change is the addition of UpdateTimeStamp(timestamp)

        o Functions UpdateTimeStamp and Update do the same thing.  The former
          is lazy about accessing the DB, while the later is not.

          - Quiz question: Why do there still need to be two functions?

    o The art service PreProcessEvent callback becomes inexpensive because it
      no longer triggers a database access.

      - The database access is now triggered by an accessor function call.

    o Existing callers of provider function Update(timestamp) might benefit
      by calling UpdateTimeStamp(timestamp) instead, but they won't break
      if they do not make this change.

  - Risks

    o During initial conversion an implementor might forget to add a call to
      DoUpdate(timestamp) in an accessor function.

  - Status

    o Code available on branch feature/greenlee_lazy_db

    o Tested with TPCEnergyCalib

    o Propose 4 IOV svcs/provider in LArSoft and 7 IOV in uboone need to be
      modified.

      - Other experiments should do this as well
    
10:15 Lorena Escudero - Pandora: Ongoing Work

  - PFParticleMetadata

    o Move from larpandora to lardataobj to avoid cyclic dependencies
    o Affects lardataobj, larpandora, dunetpc
    o Time-sensitive for uboone, should it go into this week's release?

    The pandora group thinks they can have it by tomorrow noon for this
    week's release.  Lynn Garren can do it if they complete testing in time.

  - Add Assns Slice -> PFParticle, and Assns Slice -> Hit

    o Affects larpandora, means new output data products
    o Will present at next coordination meeting and discuss it then