-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FilledBunches and CollidingBunches removed from the makeScanFileII.py #35
Comments
Hi. makeBeamCurrentFileII.py does not reqiure information on bunches from Scan_****.pkl, if you use the version from the master branch (I've checked this just now) |
Dear Anton, Thank you for your fast reply, I have a copy of master branch from June 20, 2017 (there were no updates after that yet), and I am getting an error: I have just checked the "makeBeamCurrentFileII.py" from the master branch: As for the "makePCCRateFile.py" - thank you, import of "inputDataReaderII.py" helped to avoid previous error, but there is a new one: Please let me know if you have some more ideas, |
Hi Oleksii,
It maybe that colliding is not filled. Can you add "print colliding"
before the if statement and see if it is empty or if there is indeed a
mis-match?
Cheers,
Chris
…On Thu, Aug 3, 2017 at 3:53 PM, oturkot ***@***.***> wrote:
@anbabaev
Dear Anton,
Thank you for your fast reply, I have a copy of master branch from June 20,
2017 (there were no updates after that yet), and I am getting an error:
File
"/afs/cern.ch/work/o/oturkot/Lumi/VdMFramework/makeBeamCurrentFileII.py",
line 321, in doMakeBeamCurrentFile
CollidingBunches = scanInfo["CollidingBunches"]
I have just checked the "makeBeamCurrentFileII.py" from the master branch:
https://github.com/CMS-LUMI-POG/VdMFramework/blob/master/makeBeamCurrentFileII.py
and it also has "CollidingBunches = scanInfo["CollidingBunches"]" in line
321.
As for the "makePCCRateFile.py" - thank you, import of
"inputDataReaderII.py" helped to avoid previous error, but there is a new
one:
Problem in makePCCRateFile_config.json, specified PCC_BCID not among
colliding bunches specified in scan file, exit program
which is line 213 in:
https://github.com/CMS-LUMI-POG/VdMFramework/blob/master/dataPrepII_PCC/makePCCRateFile.py
Please let me know if you have some more ideas,
Thank you,
Oleksii.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Hi Ah, Indeed. Somebody added function getCurrentsFromTimber and did not check that the Framework works after this. We uses the version before the function getCurrentsFromTimber has been added. |
@gkrintir Can you help debug this since you added the Timber functionality? We need a patch for this ASAP. |
Hello, @anbabaev can you paste which configuration are you using ? |
Hi.
|
|
Ah ok I see, thx. I missed this because I didn't generate one scan file from scratch. Yes, please comment out these lines and don't use the Timber function for the moment. But I don't see the reason why the info about bunches were REMOVED from the scan file. This completely breaks the flow with previous configurations. The proper way is to define a separate way of reading the bunches per scan point, if user wishes. And this should be configurable and not hard coded. @capalmer85 am I missing smth here ? |
Do not worry. The 'new' version is fully compatible with old fills. I've tested the code with 2015-2016 data starting from fill 4266. |
Yes, that's a very nice and meticulous kind of work. However, imagine that I want to reproduce an old result (buggy or not) so as to compare it with all the fixs you suggest. With this commit it gets impossible, right ? Also, do we know why this happens ? It's worrisome information that should be available per fill is not available for all scans. This points to problems with DIP protocol. I think that the framework should work for some time with both configurations i.e, "old" (before your commit) spitting out the warnings and "new" (after your commit). We should understand whether the results are different, how&why, and only then fully migrate to "new". Was any validation test performed before the migration, e.g. compare x-section results ? If so which fill(s) ? |
All these have been discussed earlier. I am not a github moderator and can not change the master branch by my own will :)
|
Thanks Anton, that's re assuring then. Yes (1) e.g. happened during pPb 2016 period due to FBCT failure. That's why we wrote the Timber function to read per bunch values from BQM. Ok, what I suggest then is to simply put back to makeScanFileII.py the info about FilledBunches and CollidingBunches. This should be anyway independent from the way we later fill the list of bunches. I ll prepare the commit. |
Dear All, Thank you a lot for fast and useful replies, after commenting out lines 321-323 the "makeBeamCurrentFileII.py" works. I have checked the workflow for PCC luminometer without corrections, and it looks like that only "dataPrepII_PCC/makePCCRateFile.py" needs to be updated to work with new implementations (I assume makeRate scripts for other luminometers might need similar updates). At the moment the rate files stored in the master branch can be used. |
Hi all, simply putting the info about bunches in scan file: #36 solves the issue for both Timber and dataPrepII_PCC/makePCCRateFile.py. As a reminder (as stated in README), if you want to enable the Timber function you have to specify it in the makeBeamCurrentFileConfig fragment by "ReadFromTimber" : true option. |
@anbabaev
@capalmer85 @karacheb
In the commit 58cd981 FilledBunches and CollidingBunches were commented out from the makeScanFileII.py, and after that removed.
But these information is still required, e.g by
dataPrepII_PCC/makePCCRateFile.py
makeBeamCurrentFileII.py
Was it done intentionally ? If yes, what to do with scripts which require these information from the Scan_****.pkl file ?
The text was updated successfully, but these errors were encountered: