alcap-org / AlcapDAQ

Alcap DAQ
alcap-org.github.io
8 stars 0 forks source link

PlotTAP_TDiff produced no plots #221

Closed AndrewEdmonds11 closed 10 years ago

AndrewEdmonds11 commented 10 years ago

This could be an obvious mistake by me but I want to put it here so that when I find the solution others will learn from my mistake.

benkrikler commented 10 years ago

My guess is this is related to #220. Is MakeDetectorPulses still creating TDPs ok and filling the gDetectorPulseMap?

AndrewEdmonds11 commented 10 years ago

Ok, I think I've found the problem for #220 .

A lot of the channels are being analysed with the PassThroughDPGenerator (rather than MaxTimeDiff) and so PlotTDPs is ignoring them.

Isn't the use of PassThrough hard-coded so that only muSc/NDet etc. use them? There's nothing in the production.cfg that seems to be setting these generator.

benkrikler commented 10 years ago

The PassThrough generator is used to pass un-pairable channels through to give TDPs with only one channel filled. Unpairable is defined (and was implemented) to be any detector that has both a fast and a slow channel. The check was made using the IDs::channel::GetCorrespondingFastSlow which returns the original ID if the corresponding fast / slow channel doesn't exist. That should be the case for all scintillators and the NDets but not so for any Silicon or Germanium detector channels.

As well as this condition, both channels must have been analysed at the previous step. If you're specifying each channel individually for MakeAnalysedPulses is it possible that in your modules file you're only specifying the fast or slow part (or ommitted that part in the channel name)?

AndrewEdmonds11 commented 10 years ago

Thanks for the explanation, Ben. I see now how the two problems could be linked.

is it possible that in your modules file you're only specifying the fast or slow part (or ommitted that part in the channel name)?

Nope, each channel as the fast/slow suffix:

SiL1-1-S=FirstComplete(0.50,true)
SiL1-2-S=FirstComplete(0.30,true)
SiL1-3-S=FirstComplete(0.40,true)
SiL1-4-S=FirstComplete(0.70,true)

SiL1-1-F=FirstComplete(0.30,true)
SiL1-2-F=FirstComplete(0.10,true)
SiL1-3-F=FirstComplete(0.40,true)
SiL1-4-F=FirstComplete(0.90,true)

Ge-S=FirstComplete(0.60,true)
Ge-F=FirstComplete(0.90,true)

NDet=FirstComplete(0.50,true)
NDet2=FirstComplete(0.50,true)
ScR=FirstComplete(0.50,true)
ScL=FirstComplete(0.50,true)
ScGe=FirstComplete(0.50,true)
ScVe=FirstComplete(0.10,true)

I'll go a step back and look to make sure the TAPs are being generated.

AndrewEdmonds11 commented 10 years ago

Ok, I think I know the problem with issue #220.

MakeDetectorPulses tries to pair sources together, which can be different based on the generator options.

Here's a concrete example, where I output the given channel, the given generator and the predicted partner:

AE: Ge-F, FirstComplete#{constant_fraction=0.90}{no_time_shift=true}, Ge-S#FirstComplete#{constant_fraction=0.90}{no_time_shift=true}
AE: Ge-S, FirstComplete#{constant_fraction=0.60}{no_time_shift=true}, Ge-F#FirstComplete#{constant_fraction=0.60}{no_time_shift=true}

and then on the following line, we check to see if that source exists and if it doesn't to just set it to be the same as the one given:

if(gAnalysedPulseMap.count(partner)==0) partner=i_source->first;

So the Germanium channels are not being matched because they have different constant_fraction values.

Any idea of the best way to solve this?

PS I don't think this explains why PlotTAP_TDiff isn't producing plots.

benkrikler commented 10 years ago

We need to ignore the options given to a generator I suppose. You could implement this yourself if you just check aSource.Generator().Type()==anotherSource.Generator().Type().

It would be nicer to put this into the source and generator ID classes though. Maybe a matchesUptoOptions method that only checks the channel and Generator Type match.

AndrewEdmonds11 commented 10 years ago

Thanks, Ben. I'll check that fix.

As a gut feeling, I'd be a bit worried that we end up with too many match functions and then no-one knows which one to actually use for a given situation. But maybe it would be better...

AndrewEdmonds11 commented 10 years ago

So the problem with the fix is that, although it's fine to check that the generators match. The new source generated still doesn't actually exist in the gAnalysedPulseMap.

i.e. (First line is same as above and the second is the "types" of the two generators)

AE: Ge-F, FirstComplete#{constant_fraction=0.90}{no_time_shift=true}, Ge-S#FirstComplete#{constant_fraction=0.90}{no_time_shift=true}
AE: FirstComplete, FirstComplete

This Ge-S source doesn't actually exist. Is there a way to get the correct Ge-S source (Ge-S#FirstComplete#{constant_fraction=0.60}{no_time_shift=true} from the given Ge-F source?

AndrewEdmonds11 commented 10 years ago

Ok, I now just loop through the gAnalysedPulseMap again to find the corresponding fast/slow channel:

// Find the correct source since the generator options could be different
        for (SourceAnalPulseMap::const_iterator j_source = gAnalysedPulseMap.begin();
             j_source != gAnalysedPulseMap.end(); ++j_source) {

          if (j_source->first.Generator().Type() == gen->Type()
              && j_source->first.Channel() == ch->GetCorrespondingFastSlow()) {
            partner = j_source->first;
          }
        }

PlotTDPs now produces plots for all the detectors.

I will implement this as a hotfix (and close #220).

This hasn't actually solved the problem with PlotTAP_TDiff but it seems some are producing plots (which they may have done before since I didn't check every single one...)

jrquirk commented 10 years ago

Speaking to the original problem, things are plotted against eachother only if they have the same generator (config string included, meaning same constant fraction choice; this can be fixed I think by removing these lines.

AndrewEdmonds11 commented 10 years ago

So the problems were related!

So I looked at the plots that were being produced and found the following combinations were good (constant fraction of both channels in brackets):

I will try John's fix and see if that solves it.

jrquirk commented 10 years ago

I hadn't even read everything before. It looks like they're exactly the same issue.

I think the things we're trying to prevent by having gen1==gen2 can be negated by making sure to only use a single gen for each channel.

AndrewEdmonds11 commented 10 years ago

Now solved with a hotfix. Closing.

litchfld commented 10 years ago

In reference to this and other bugs, I don't think it makes much sense (in general) to consider versions of the same generator with different options to be more similar to different generators. We could - logically, if not morally - have a VeryFlexibleGenerator that has multiple strategies that gave results more different from each other than one was from a different generator. Special cases, like the Constant Fraction method, have a continuum of values - but since that is a special case, perhaps depending on it should be limited to specialist pieces of code.

In any case the required code is not hard to write idiomatically:

//Make a generator mask for channels we want too match to.
Ids::Generator gen_mask(anotherSource.Generator(), kAnyConfig);
//[...]
aSource.matches(gen_mask);