# Presentation: 470668

## Muon Code Review Concrete suggestions (2)

- Paul Balm
- February 3, 2000
- (an updated version of last week’s slides)

## Outline

**General comments**
***using***** in headers**
***MuoDataProcessor***
**what’s  *****fillme***
**miscellaneous**
**MuoHitReco**
**the reconstructor in *****muo_hitreco/MuoHitReco.cpp***
**the algorithm class in *****muo_dataprocessor/MuoHitProcessor.cpp***
**MuoHitCollection**
**Hints from the *****muo_segmentreco***** towards the new design**
**Summaries & Conclusions**

## Stop using in headers

**Quick check**** (“****grep using muo_*/muo_*/*.hpp****”) ****shows using in:**
**muo_data/MuoHitCollection.hpp**
**muo_hitreco/MuoHitChunk.hpp**
**muo_segmentreco/MuoSegmentChunk.hpp**
**muo_segmentreco/MuoSegmentProcessor.hpp**
**muo_segmentreco/MuoSegmentReco.hpp**
**muo_trackreco/MuoTrackProcessor.hpp**
**muo_trackreco/MuoTrackReco.hpp**
**Fix should be easy****, but I need to proper testing and this takes time (for me it does)**

## MuoDataProcessor -  General

**Remove use of ****Event**** as argument**
**Should only be using ****RawDataChunk, MuoHitCollection, MuoSegment****, etc.**
**Move few lines of code to get chunk out of event to reconstructor package**** - trivial if you know how to test it...**
**The new design ****must allow**** for the fillme technique **(*see the Muon Code Review)*

## Definition of “fillme ”

**To pass on by reference the object to be created (or filled) as an argument to the function**
**Example: **
**Not, as it is now: **
**	****MuoHitCollection* processEvent(UnpDataChunk, error){**
**	****... **
**	****return new MuoHitCollection(...); }**
**But in stead:**** **
**	****void processEvent(UnpDataChunk, MuoHitCollection&){ **
**	****...**
**	****return; }**

**I also removed the error flag, throw exceptions instead. **

## Miscellaneous

**Memory leaks in ****muo_hitreco**
**Hits are new’ed each event **
**Never**** deleted?!?**
**All ****muo_data****-objects should ****and will**** be declared in the ****namespace ****Muon**
**This will solve compatibility problems with Xwindows’ Position object**

## MuoHitReco.cpp (1)

**Current implementation of processEvent() method:**
**Result MuoHitReco::processEvent(Event& event){  **
**ErrorLog errlog;****  **
**int error = 0;****  **
**MuoHitCollection *hitCollection = **
**MuoDataProcessor< ****Event****, MuoHitCollection>:: processData( event, error );  **
**if ( error ) return Result::success;****  auto_ptr<MuoHitChunk> chunk( ****new**** MuoHitChunk( *hitCollection ) );  **
**insertChunk(event, ****chunk****); **
**return Result::success;**
**}**

## MuoHitReco.cpp (2)

**What’s wrong/****some proposed resolutions**:
**Event**** is not a physical object and should not be used as argument of MuoDataProcessor; ****change to MuoDataProcessor <RawDataChunk, MuoHitCollection>**
**use of ****new****; ****implement fillme-methods ****(easy?)**
**did I miss any misuse of ****Thandle<>****?**
**always returns ****Result::succes**
**doesn’t use ****Errorlogger**

## MuoHitReco.cpp - fillme

**Result MuoHitReco::processEvent(Event& event){   **
**int error = 0;**
**TKey< UnpDataChunk *chunk;****	**
**UnpChunkSelector selec(D0MCH::MUO_FE);****	****const TKey<UnpDataChunk> unpKey(selec);**
**THandle<UnpDataChunk>**
**	****unpchunk=unpKey.find(event);**
**insertChunk(****unpchunk, **
**	****auto_ptr<MuoHitChunk>(**
**	****MuoDataProcessor< **
**	****Thandle<UnpDataChunk>, **
**	****MuoHitCollection>:: **
**	****processData( event, error )****  ****)  ****)****;**
**if ( error ) return Result::success;**** **
**return Result::success;**
**}**

**This can be applied to other pieces of code also!**

## MuoHitProcessor<Event, MuoHitCollection>

**Calls ****MuoDataProcessor<Event, UnpDataChunk>**
**shouldn’t exist****, this functionality should be in ****MuoHitReco**
**it assumes an UnpDataChunk exists in the Event**** (how does the user know that?)**
**Calls ****MuoDataProcessor<UnpDataChunk, MuoHitCollection>**
**use Thandle<UnpDataChunk>**
**but the algorithm should reside in another class**** e.g. to allow configuration with RCP**

## MuoHitProcessor<UnpDataChunk, MuoHitCollection>

**Create ****vector<PDTChannel>**** (also for MDT’s)**
**Loop over ****Module****’s**
**get type;****Module::getPDTChannelVector()**** will be useful**
**get channels from Module and fill channelvector**
**Call ****MuoDataProcessor<vector<PDTChannel>, MuoHitCollection>**** (also for MDT’s)**
**merge ****MuoHitCollection**** from PDT’s, MDT’s**
**return new MuoHitCollection(...)****- **
***	******fillme *****technique would prevent this**

**Will be/ Is implemented by Mike Fortner**

## MuoHitProcessor<vector<PDTChannel>, MuoHitCollection>

**Loop over channels in the vector**
**Get PDT**** (geometry object, have to get geometry first)**
**Drifttime-distance conversion**
**Figure out hit-position**** in D0 coordinates**
**Insert hit in MuoHitCollection****:**
**hitcoll->insertWire**
**(index, ****new**** MuoWireHit( position, index, ... );**
**return**** ****new MuoHitCollection(hitcoll)**

**move this functionality to ChannelAdaptor?**

**Here *****fillme***** would ****not work**
**but it would after some alterations to *****MuoHitCollection***

## MuoHitCollectioninsertWire

**In ****MuoHitProcessor<vector<PDTChannel>, MuoHitCollection>**** *****fillme***** ****would work**** if ****insertWire**** ****would take a ****MuoWireHit**** in stead of a ****MuoWireHit*******
**Drawback (one more copy) is minimal ****compared to what is gained**

## MuoHitCollectionThe Map

**Segmentfinder loops over all elements of ****map<MuoIndex, MuoWireHit*>**** ****from ****MuoHitCollection**
**It uses index and hit info together****:**
**	****the index:****	****element.first**
**	****the hit:****	****	****element.second**
**It seems to be only ****waste of space**** to not use a vector of hits, as MuoIndex is also stored in MuoWireHit: **
**	****hit.getMuoIndex()**** will give it to you**
**Currently the ****MuoIndex**** is passed on/copied ****twice****...**

## MuoHitCollectionStore Hit ’s instead of Hit* ’s

*MuoHitReco* new’s *Hits* every event and stores *Hit**’s in a *MuoHitCollection* also created with new
*MuoHitCollection* does not get deleted
- If *MuoHitCollection* would be deleted, the *Hit*’s wouldn’t! (only their pointers)
- Resolution: Store actual *Hit *objects in MuoHitCollection

## Summary MuoDataProcessor

- Change
- 	processEvent(UnpDataChunk, error) to processEvent(Thandle<...>, MuoHitCollection&)
**Use Thandle<>**
**Throw exceptions**
**Use *****fillme***

## Summary MuoHitCollection

**Store Hits, ****not their pointers**
**Store them in vectors, ****not in maps**
**Change insertWire(Hit*********) to insertWire(Hit)**

## Conclusions

**Using**** should be removed ****asap **
**see yesterday’s d0rug e-mail**
**I will take care of that next week**
**We know what to do about many of the ****memory management issues **
**largely redesign of *****MuoDataProcessor***** and *****MuoHitCollection***
**I can do it, but after the March 15 Milestone because of level 3 commitments**
**Onne is author of much of the code and would do it faster (with less mistakes?), he’ll be back April 15**