Introduction
http://www-glast.slac.stanford.edu/software/core/minutes_11062007.htm
https://confluence.slac.stanford.edu/download/attachments/20011/CalDigi+variable+readout.pdf
TriggerAlg::tracker and TriRowBitsAlg
The code to compute the 3inARow condition based on digi is duplicated between these 2 pieces of code. This is even dangerous, as everything in TriggerAlg relies on the local computation which ends up in EventHeader, while it is the version in TriRowBitsAlg that actually makes it to the TDS and then to the L1 ROOT object.
What should be done :
1) reverse TriggerAlg and TriRowBitsAlg in the basicOptions.txt
2) remove the code in TriggerAlg::tracker and replace by a call to the TDS.
User defined properties should always take precedence : exemple with WindowMask
The current snippet is
/// Apply window mask. Only proceed if the window was opened / /// or any trigger bit was set if window open mask was not available.
if (m_applyWindowMask){
unsigned windowOpen=0;
if (m_pcounter)
{ /// using ConfigSvc
/ windowOpen=trigger_bits & tcf->windowParams()->windowMask();
}else{
windowOpen=trigger_bits&m_mask;
}
if ( \!windowOpen){
m_window_reject++;
setFilterPassed(false);
return sc;
}
}
m_pcounter is the pointer to the configuration service, and m_mask defaults to 0xffffffff, id est no mask applied. If I had been able to use m_mask to define the right mask to use, I could have been able to override the missing info from ConfigSvc. So, I think that a more flexible logic would be :
/// Apply window mask. Only proceed if the window was opened / /// or any trigger bit was set if window open mask was not available.
if (m_applyWindowMask){
unsigned windowOpen=0;
if (m_pcounter&&m_mask==0xffffffff){ /// using ConfigSvc/ windowOpen=trigger_bits & tcf->windowParams()->windowMask(); }
}else{
windowOpen=trigger_bits&m_mask;
}
if( !windowOpen)
{
m_window_reject++;
setFilterPassed(false);
*return* sc;
}
}
1) ConfigSvc not available : m_mask is used
2) ConfigSvc available and mask is default pass_all : the user did not set a specific mask and the service is there, so rely on the service
3) ConfigSvc available but mask is not default : the user expresses an intent, as to what he/she wants to applied, so use his/her mask and not the one provided by ConfigSvc.
8 Comments
Kelly (Arrighi), Heather
Just want to confirm intended TriggerAlg behavior.
current cal portion of TriggerAlg code is as follows.
// CAL CASE 1: Get bits from existing GltDigi class
if( glt!=0 )
// CAL CASE 2: Populate new GltDigi class with info from CalTrigTool
else if(cal!=0){
//<-- this means, generate cal trigger bits from simulated CalDigis
I propose to re-replace this code with logic like this
// CAL CASE 1: Get bits from existing GltDigi class
if( glt!=0 )
}
// CAL CASE 2: Populate new GltDigi class with info from CalTrigTool
else {
//<-- no longer dependent on CalDigis, will work directly from MC data instead.
//<-- My code will fall back to CalDigis if MC is not present.
Remind me, are we still processing CalDigis for real data? If so, is (glt!=0) the right test for real vs MC processing?
thanks,
zach
Kelly (Arrighi), Heather
I had to add the " else if(cal!=0){"
so that TriggerAlg would not crash when it is run in the
Interleave branch because apparently Zach's code does not protect against
missing TDS information. If the new version of the code does
not crash in this way in interleave any more then the line can go back
to just "else{".
Martin
Kelly (Arrighi), Heather
Hello Johann,
The end goal is to allow cal digi readout mode (bestrange, zero-suppression) to vary with trigger, as it does in real life.
This more or less requires that CalDigiAlg run after TriggerAlg, which means that TriggerAlg needs to work from MC & not from CalDigis.
I committed code back in Janurary which allows Cal trigger code to work in multiple modes (off of MC or digi, before or after TriggerAlg).
We are now implementing it & experiencing some (apparently minor) glitches...
clear(er)?
-zach
Kelly (Arrighi), Heather
CalTrigTool currently returns failure code under the following condition: there is neither MC data in TDS nor CalDigi data. I did not know at the time that I wrote it that this case would be a valid condition.
What exactly is the proper behavior in this case? Return zero for all cal trigger bits? Or just do nothing?
So I want to get all this stuff right & I'm a little overwhelmed, I don't fully understand all the use cases for Cal portion of TriggerAlg.
Can someone set me straight?
For example:
copy cal trigger bits in GltDigi to trigger_bits var (why am I doing this???)
else if MC exists
calculate Cal trigger bits from MC & save to trigger_bits var
else if CalDigi exists
calculate Cal trigger bits from CalDigi & save to trigger_bits var
create & register GltDigi object ?????
else
do nothing??? set cal portion of trigger_bits to zero???
Anyways, this stuff should be pretty easy to implement once I know what I'm doing.
-zach
Kelly (Arrighi), Heather
Since the crashes can be bypassed by a few lines in TriggerAlg
and we needed results quickly we just patched TriggerAlg but maybe
it would be good to move the fix to CalTrigTool instead. For the
interleave case it would not matter whether your function set all
trigger bits to 0 or just did nothing.
The interleave instance of TriggerAlg only does livetime calculation.
It does not have any subsystem digi information available because the
interleaved events do not get reconstructed so none of the digi code
gets run there. So TriggerAlg needs to be able to run without those
TDS objects.
Martin
Kelly (Arrighi), Heather
If I understood Zach correctly then the code is already split up
in a way that CAL digi does not need to run before TriggerAlg.
TriggerAlg as such does not need CAL digi, it uses Zach's
CalTrigTool. The only use that TriggerAlg has for CalDigi is
to prevent the crashes in interleave mode but if Zach changes
his code then we can get rid of CalDigi in TriggerAlg.
Martin
Kelly (Arrighi), Heather
This is correct, I already did all the hard work. The only problem is that I still don't fully understand the Cal portion of TriggerAlg & want to make sure I know exactly what I'm doing before I commit. Here's what I have so far, and a couple questions still remain....
from johann:
> I think we have the Glt info for digi-reprocessed trigger conditions and Gem info for the real stuff.
from Martin:
> For the
interleave case it would not matter whether your function set all
trigger bits to 0 or just did nothing.
ok, so now I'm getting a better understanding.
here's an updated pseudo-code, remaining questions in bold.
if MC exists // we are in an MC type run
calculate Cal trigger bits from MC & save to trigger_bits var
else if CalDigi exists // we are in a readdigi_runrecon type run
calculate Cal trigger bits from CalDigi & save to trigger_bits var
save trigger bits to GltDigi (create it if it doesn't exist? i really think someone else should be creating it.)
else // interleave mode
set bits to zero.
But I still don't understand why we are currently reading from the GltDigi bits in TriggerAlg::calorimeter().
-zach
Kelly (Arrighi), Heather
I have modified TriggerAlg & CalXtalResponse to what I think should work (general strategy was to change as little as possible).
But I still don't understand what TriggerAlg is supposed to be doing, is any one able to answer my questions?
1. Is pseudo-code below correct?
2. Which operating mode does presence of GltDigi in TDS before TriggerAlg indicate? (currently there is a test for this & it's not consistent with my understanding)
3 Under which circumstances (if any) do we create®ister GltDigi inside of TriggerAlg & can we please move it out of cal portion of code?
Thank you,
Zach
if MC exists // we are in an MC type run.
calculate Cal trigger bits from MC & save to trigger_bits var
(remember that TriggerAlg can no longer depend on CalDigi for MC simulations.)
else if CalDigi exists // we are in a readdigi_runrecon type run
calculate Cal trigger bits from CalDigi & save to trigger_bits var
save trigger bits to GltDigi (create it if it doesn't exist? I really think someone else should be creating it.)
else // interleave mode
do nothing