 |
 |
Fine, remove the class. It's not being used right now (the class was written when there wasn't a base implementation of RawCalorimeterHit, and I removed most usage after BaseRawCalorimeterHit was added), so it's good housekeeping. But I disagree with your reasons. If I were still using this class, I would argue to keep it in. I agree that nobody should be using non-base classes to retrieve event information from LCIO. I don't see why that means you need to purge the codebase of non-base classes. People who use non-base classes in their event.get() calls will get errors immediately when that driver is run from LCIO - no unpredictable behavior, and half a dozen people on the software list will recognize the error and know how to fix it. This is kind of the point of type safety. Of course we need to discourage people from using non-base classes in code that might need to run from LCIO, but that's a problem you solve by documentation and user education - put a note in the class javadoc and/or put the class in a package specific to its use (make it clear the class is for readout simulation, not recon). I particularly don't understand why ECal code is being held to such puritan standards when the SVT code is obviously never going to reach those standards. SVT code relies heavily on hit and track subclasses that cannot be recovered from LCIO. And people understand that, and we live with it. On Fri, 9 Jan 2015, McCormick, Jeremy I. wrote: > Okay.... > > But the problem here is that any downstream analysis Driver in a separate job which tries to retrieve event information using this (public) class will fail. Therefore, I believe it should be removed, in favor of using the base class from LCSim. > > And as far as I can tell there is only one reference in our entire code base that uses the window size information, which is just an analysis plot. > > So can the class simply be removed? > > I assume you have some other way of getting the window size or that it is simply a hard-coded parameter. (Not sure, maybe you can clarify for me where this information comes from in the first place.) > > --Jeremy > > -----Original Message----- > From: Sho Uemura [mailto:[log in to unmask]] > Sent: Friday, January 09, 2015 12:05 PM > To: McCormick, Jeremy I. > Cc: hps-software > Subject: Re: HPSRawCalorimeterHit > > The window size there is only used for MC studies, and it doesn't need to be persisted. > > On Fri, 9 Jan 2015, McCormick, Jeremy I. wrote: > >> Hi, Sho. >> >> In the ECAL code, is there another way we could keep track of the window size rather than extending an LCIO class? >> >> e.g. >> >> ecal-recon/src/main/java/org/hps/recon/ecal/HPSRawCalorimeterHit.java >> >> If this is something that is the same for all hits in the collection, then it would be possible to use a setting on the collection itself rather than each hit. >> >> Or is this value specific to each hit? >> >> As far as I can tell, the added method getWindowSize() is not even used anyplace in the reconstruction, so can it simply be removed? >> >> --Jeremy >> >> ###################################################################### >> ## >> Use REPLY-ALL to reply to list >> >> To unsubscribe from the HPS-SOFTWARE list, click the following link: >> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=HPS-SOFTWARE&A=1 >> > > ######################################################################## > Use REPLY-ALL to reply to list > > To unsubscribe from the HPS-SOFTWARE list, click the following link: > https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=HPS-SOFTWARE&A=1 > ######################################################################## Use REPLY-ALL to reply to list To unsubscribe from the HPS-SOFTWARE list, click the following link: https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=HPS-SOFTWARE&A=1