search

JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

Scalerfix1 #178

Closed rwmichaels closed 5 years ago

rwmichaels commented 5 years ago

There are two changes here which we need for the "official" analyzer on the counting house computers. These pertain to Scaler analysis.

  1. THaScalerEvtHandler must allow for inheritance.
  2. GenScaler had a problem that if it was not fed the header word for the first word, it would do something wrong. I think Tritium must have fixed these things privately, as I see in a "TriScaler" library.
hansenjo commented 5 years ago

The change from private to protected access in THaScalerEventHandler breaks binary compatibility. If it is not too hard, could we work around that by adding protected Get/Set methods to access the information your derived class needs? I can add that quickly myself. Could you send me the source of the derived class?

rwmichaels commented 5 years ago

The derived class replaces the public "Analyze" and uses EVERYTHING else from THaScalerEventHandler, so basically

a (slightly) different analysis. I don't mind sending it, but why is "binary compatibility" important ? I don't get it. It's a

new experiment, so a new code; also it's an improvement which still works the old way. Anyway, I'll send the derived

class privately and maybe you can figure out a different/better way.

From: Ole Hansen notifications@github.com Sent: Friday, May 3, 2019 11:45:24 AM To: JeffersonLab/analyzer Cc: Robert Michaels; Author Subject: Re: [JeffersonLab/analyzer] Scalerfix1 (#178)

The change from private to protected access in THaScalerEventHandler breaks binary compatibility. If it is not too hard, could we work around that by adding protected Get/Set methods to access the information your derived class needs? I can add that quickly myself. Could you send me the source of the derived class?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://gcc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FJeffersonLab%2Fanalyzer%2Fpull%2F178%23issuecomment-489141642&data=02%7C01%7Crom%40jlab.org%7Cafd2be57aaf445bfa44a08d6cfde5c0f%7Cb4d7ee1f4fb34f0690372b5b522042ab%7C1%7C0%7C636924951271606171&sdata=lrHYTE4EdePia9AvrgDnIc9szYTJUwRyaxU8xXAI%2FO0%3D&reserved=0, or mute the threadhttps://gcc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAUBLRFTYWHQXARKP7M2223PTRMZJANCNFSM4HKUK4QA&data=02%7C01%7Crom%40jlab.org%7Cafd2be57aaf445bfa44a08d6cfde5c0f%7Cb4d7ee1f4fb34f0690372b5b522042ab%7C1%7C0%7C636924951271616184&sdata=EU%2F3uGUX8Izquj6Dxp%2B2QKOQ%2BafJizfxzSdZTFWx9NU%3D&reserved=0.

hansenjo commented 5 years ago

Merged with commit c9962e8de57f62dba7be96e36a1d9cdd99406886 on master. As discussed with Bob, we'll only merge the change to GenScaler, which is binary-compatible, into the 1.6 branch, and his private scaler event handler will be changed form a derived class to a reimplementation.

hansenjo commented 5 years ago

I have applied a hotfix for the GenScaler code to the installation in the counting house. (You'll still see version 1.6.6, but "analyzer --version" now says "git @e772ec1".)