Updated toa_vref_scan for multiple rocs#428
Conversation
tomeichlersmith
left a comment
tomeichlersmith
left a comment
There was a problem hiding this comment.
I have some feedback on the algorithm and removing some deadcode.
More generally, I'm going to need more evidence that this is working. Maybe some before/after comparisons of the HGCROC parameters? Even better, some before/after plots of the TOA as a function of channel and calib pulse height. I can help guide you on how to make these comparisons if you want. What did you run that made you say it was "tested ... successfully"? These extra details are crucial for making sure the code is working as intended.
Plotted TOA vref vs efficiency with data aligning with toa_vref_scan's formula of highest_non_zero_eff + 10
|
tomeichlersmith
left a comment
tomeichlersmith
left a comment
There was a problem hiding this comment.
Good progress - I am still concerned about if this is actually determining the toa parameters (toa_vref and/or trim_toa) for all of the active ROCs or if this is just doing them for the currently selected iroc. Maybe share the terminal output of the toa_vref_scan while printing out the settings ("View deduced settings")?
You also "resolved" some of my comments without changing the code or responding. If you are not going to be able to update the code to address my comments please respond to them saying why you aren't going to do those updates at this time.
|
I am merging |
two methods of obtaining toa_vref values; 1) traditional method, by scanning 255-0 vref and then finding the highest_non_zero efficiency value and then 2) faster method, scans starting at 255 and going down until it finds the first non-zero efficiency value for each link in each roc, then setting that as the toa_vref value;
both of these methods yield identical results, with option 1 being required to plot the data distribution and option 2 being a much more efficient way to obtain data |
plots for toa_vref vs efficiency for each roc from this run (in order rocs 0, 1, 2, 3)
|
| description='takes a csv from tasks.toa_vref_scan and outputs a plot of TOA efficiency per channel against TOA VREF values' | ||
| ) | ||
| parser.add_argument('-f', required = True, help='csv file containing scan from tasks.toa_vref_scan') | ||
| parser.add_argument('-r', '--roc', type = int, choices = [0, 1, 2, 3], required = True, help = 'ROC index to filter data (e.g., 0, 1, 2, 3)') |
There was a problem hiding this comment.
The EcalSMM can have up to 6 ROCs, so this should allow the indices 4 and 5 as well.
| sys.exit() | ||
| data, head = read_pflib_csv(args.f) | ||
|
|
||
| data = data.iloc[args.roc::4].copy() |
There was a problem hiding this comment.
This selection only works when you know there are 4 active ROCs and worse it fails this assumption silently!
Please include the i_roc in the CSV that's written by toa_vref_scan so that you can just select on it after loading the data into a dataframe (e.g. with data[data.roc == args.roc]). Then if you try to plot roc 2 but roc 2 data was not collected, then your plot will be empty.
There was a problem hiding this comment.
working on this now, will have this by tomorrow
| << ", getting max efficiency per link"; | ||
|
|
||
| // Save raw channel efficiencies to .csv file | ||
| csv_file << toa_vref; |
There was a problem hiding this comment.
| csv_file << toa_vref; | |
| csv_file << toa_vref << ',' << i_roc; |
…eaned up commented code, and fixed non-4 roc bug with toa_efficiency.py
tomeichlersmith
left a comment
tomeichlersmith
left a comment
There was a problem hiding this comment.
Looking good :) glad to hear you are working on the plotting. I found that the i_roc index was not being written out in the current version of the code so that might need to be patched before the plotting can work.
I'd also request that the writing of the CSV file is optional or I can at least configure where the output file is.
| // create a .csv file to save efficiency and vref data for analysis | ||
| auto now = std::chrono::system_clock::now(); | ||
| auto in_time_t = std::chrono::system_clock::to_time_t(now); | ||
|
|
||
| std::stringstream ss; | ||
| ss << "toa_vref_scan_data_" | ||
| << std::put_time(std::localtime(&in_time_t), "%Y%m%d_%H%M%S") << ".csv"; | ||
| std::string filename = ss.str(); | ||
|
|
||
| std::ofstream csv_file(filename); | ||
| pflib_log(info) << "Saving scan data to file: " << filename; | ||
| csv_file << "TOA_VREF,ROC"; | ||
| for (int chan = 0; chan < 72; ++chan) { | ||
| csv_file << "," << chan; | ||
| } | ||
| csv_file << "\n"; |
There was a problem hiding this comment.
Have the filepath be given as an argument to this function. Then the task that calls this function can have the user put in their desired output file (use pftool::readline_path). An empty string can signal that the user doesn't want the CSV file written at all (which will be helpful in the long term when we are tuning many chips and have more trust that the algorithm is working as intended).
There was a problem hiding this comment.
We can also achieve the "not writing out" by always writing out to some ofstream and then using the special file "/dev/null" to throw away data when we don't want it.
|
several things have been updated with this most recent commit: 1. added a yes/no option before running toa_vref_scan on whether or not to write the data to a csv file (for plotting purposes): 
 '
these 3 pictures show that each of the options work and achieve desired results (when saying no to writing to a csv file, there is no print statement showing that it is not writing to the file, as well as it not outputting one after running the scan); the point of this is for efficiency and not writing unnecessary files later down the road. 2. updated the methodology of the toa_vref_scan to be more efficient: 
- second method (the updated) starts scanning at vref = 255 and loops down until finding the first non-zero efficiency value for each link in each active roc (and then adding 10 to set the reference voltage), thus saving almost 50% of the runtime as it doesn't need to loop all the way to 0: 
3. as long as a csv file is written for this scan, the data can be plotted: - these 2 pictures are for when rocs 0 and 1 were scanned and the accompanying data was plotted (note this is the typical distribution found across all tests): 
- these 2 pictures are again for rocs 0 and 1 but when the option to not scan all values was selected (the updated scan), these plots are less useful but it is still worth noting that the python file has functionality for both options: 
|
tomeichlersmith
left a comment
tomeichlersmith
left a comment
There was a problem hiding this comment.
Excellent work 💯 thank you for all of these updates, this looks great!
3 participants
Updated toa_vref_scan and get_toa_efficiencies to run for multiple rocs (tested on all 4 successfully)