-
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
Summary text assumes quartets #240
Comments
japan/Analysis/src/QwPromptSummary.cc Line 159 in 7580f72
|
Yeah. It assumes quartets. Downside of hardcoding. Probably do this through a configuration file. I have an update to prompt summary in the works but it's stalled because of other priorities. Will try to get it in as soon as I possibly can. |
This needs to be fixed relatively soon even if it's again hard-coded fix so that we have our charge counter works properly for the shift crew. We could live with correcting the charge counter back, but we need a fix for this and probably re-prompt for the future. |
To stop-gap hack this to work from the 240/octet time onwards (until we fix the prompt summary and respin) I've multiplied runs after 3874-> good charge by 2x in the acc_charge.py script and it shows +1.5 more Coulombs as expected. |
This was worked around during PREX, the thing that cared was our python script for getting charge into the charge plotter, but we should probably fix this for Respins so we don't have wrong information floating around. |
it's the other way around.. charge script kept assuming quartets because the summary output didn't get updated. I agree, it would be good to get it fixed for respin. |
I remember trying to solve this and then getting distracted with other stuff. I will take a look tomorrow at the branch I was working on. |
Has there been progress on this? |
Yes, I think it’s fine. I was seeing an issue with pvdb, but it was a too-aggressive cut from Sanghwa’s script that did it. |
It looks like the summary text assumes quartets and this is breaking our downstream good time, events, and charge measurements as well.
Probably we can work around this in the downstream codes, but it should be fixed here too.
The text was updated successfully, but these errors were encountered: