Djamel,
To make this calibration stuff move forward quicker let me come with some comments, not based on running the code
in any ways but just doing a walkthough the README and source code.
First, I seems nicely done following the conventions laid out, thge description in the README
together with the Brat Guide should bring all of this stuff well along.
This done let me come to the comments.
- You do not describe (or I did not find it) the default Policy for the parameters; I will suggest than unless there are good reasons
otherwise we should pick policy=-1 (i.e. if a given period is not there pick the nearest previous valid calibration) The rationale
is we do not necessarely want to calibrate all parameters for each single run.
Any comments to this?
- Specifics to cal method for pedestals. I know you did not require special triggers i.e sync event where it is guarenteed that
not real signals are there but merely that TDC is not there. This could in pricipal add to the width, move the centroid etc. It
is probably ok, but on the other hand we do not have to do pedestals cal;ibrations very often, so making the reqs a bit
more stringent does not hurt. One might also add test that the found bin fopr max adc is in fact near the <mean> and so on.
- Did you find any significant different between doing the fits, or merely take the mean and RMS of each channels. COuld it not have been
done with a profile hist?
- For the methods of the pedestal and widht it does probably not matter whatever method we use, but these comments are there to
point out this is the crucial part, and we should attempt to have the code find appropriate regions to consider etc, and not rely on
inputs setters (too much ) e.g. the FitWindow () is the default value good for 99% of the cases? ...
Options to the variuos programs:
I have now noted that we are defining these options in many utilities and even though there is overlap it is not utterly consistent.
We need to revisit these by comparing proposal for such in
a) this calibration program
b) The DbApp used to defined detectors
c) good defaults for Config.C files.
My suggestion is to assemble in a table what has been used so far and come with a proposal to define a minimum set that ALWAYS
with the Brat analysis has the same meaning e.g. -i == inputfile name -o==output filename.
-- The DB access programs
-- ReadDbTof.
-- Big Q (1) Why do you have any Db specific access inside this program. It seems to me that the ParameterElementManager + the
BrTofCalibration class should be the only place to do specific DB but you have lots of place with
e.g.
BrDbDetector *d = maindb->GetDetectorByName(...)
..
and then ending with using the ParameterElement anyhow via tofpar[i]->getTopADcGain[j].
It the reason for this that the elMan do not set errors if a given rev. is not there or why? There ought not to be any DbAccess in most user code;
You are going to have a problem with the hardcoded tofn[] =83 for tofw -it is already for the next run 125!
I do not have a straitforward solution, excpet that even now the BrDetectorParamsTof can supply this. (I am not sure this
should be in the MYSQL database - it may only change once afterall!)
Q(2)
Why do you do individual argv argc decodig rather than using the BrOpions(s). This wiill make code more consistent across
modules. Same for other programs.
-- RunInfo program
mIt is a fine program, I just want to point out that the runinfo.perl script can easily do precesily the same. This is merely a matter of taste.
-- Documentation.
The README is great for getting an overview. I strongly advocate Christians advice to state in the Module description
a) needed input
b) description of algorithms
c) produced output
This does make everybodys life easier, particular further down the line.
As always the comments takes up a lot, but again the overall scheme looks nice, and can serve as template for other applications
cheers
Flemming
------------------------------------------------------
Flemming Videbaek
Physics Department
Brookhaven National Laboratory
tlf: 631-344-4106
fax 631-344-1334
e-mail: videbaek@bnl.gov
This archive was generated by hypermail 2b29 : Wed May 30 2001 - 21:03:00 EDT