158 multi channel option for parameter timescan #179
Conversation
| @@ -26,7 +26,7 @@ | |||
| parser.add_argument('-ex', '--extra_csv_files', type=Path, nargs='+', help='time scan data, if you want to plot data from multiple csv files. Can be used to compare different parameter settings on the same plot. Adds a legend that takes the parameter settings.') | |||
| parser.add_argument('-o','--output', type=Path, help='file to which to print, default is input file with extension changed to ".png"') | |||
| plot_types = ['SCATTER', 'HEATMAP'] | |||
| plot_funcs = ['ADC-TIME', 'TOT-TIME', 'TOT', 'TOT-EFF', 'PARAMS'] | |||
| plot_funcs = ['ADC-TIME', 'TOT-TIME', 'TOT', 'TOT-EFF', 'PARAMS', 'HR_CORR'] | |||
There was a problem hiding this comment.
This PR has the same changes as #178 which is fine, we'll just merge them one-by-one.
There was a problem hiding this comment.
That was accidental, my bad!
| } | ||
| else { |
There was a problem hiding this comment.
Style, not a big deal but helpful for readability
| } | |
| else { | |
| } else { |
| bool tmp = pftool::readline_bool("Pulse into channel 61 (N) or all channels (Y)?", false); | ||
| if (tmp) { |
There was a problem hiding this comment.
No reason to manually create a tmp variable when the compiler can do it for you!
| bool tmp = pftool::readline_bool("Pulse into channel 61 (N) or all channels (Y)?", false); | |
| if (tmp) { | |
| if (pftool::readline_bool("Pulse into channel 61 (N) or all channels (Y)?", false)) { |
| (param_names[i_param].first == "REFERENCEVOLTAGE_0" ? refvol_page : | ||
| param_names[i_param].first == "REFERENCEVOLTAGE_1" ? refvol_page : | ||
| param_names[i_param].first), | ||
| "REFERENCEVOLTAGE_0", |
There was a problem hiding this comment.
This change breaks the ability for a parameter in the input parameter points file to be from a page that isn't REFERENCEVOLTAGE (0 or 1). Why did you make this change? What was the motivation?
If you need to only be changing the RV page parameters, then maybe you should make a new command instead of trying to make this (already pretty complicated) command work for your purposes?
There was a problem hiding this comment.
I understand, it takes away the flexibility. I mainly changed it to this, because I'm not using refvol_page as a variable anymore. I tried incorporating that originally, but I could not make the link variable work in the multi-channel case, and right now we were indeed just changing the RV page parameters. I don't want to take this into a separate command in case we maybe need the multi-channel option in combination with the TOT or TOA in the future, but I will think of something to make this more general.
Added a multi-channel option to the parameter timescan function, in order to study the ADC linearity for multiple channels simultaneously. It adds a column to the csv saving the channel number, and applies the same parameters to all 72 channels.