-
Notifications
You must be signed in to change notification settings - Fork 155
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
[Anchor] Use STF and ETF #1597
[Anchor] Use STF and ETF #1597
Conversation
REQUEST FOR PRODUCTION RELEASES:
This will add The following labels are available |
47d084e
to
37799e1
Compare
Use BasicCCDBManager's getRunDuration to determine time range to be covered by simulation. This will first try to get STF and ETF. If not available, the fallback chain is 1. SOX and EOX 1. SOR and EOR
37799e1
to
b37d1cb
Compare
print ("WARNING: Inconsistent EOR information on CCDB (divergence between GRPECS and RCT) ... will take RCT one") | ||
EOR=rct["EOR"] | ||
if run_start is not None: | ||
print (f"INFO: GRPECS SOR ({SOR}) will be superseded by externally provided run start ({run_start})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not anymore from GRPECS, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as seen by this function, the value from GRPECS
will be overridden.
We can simply do that silently. Or make the output more clear that all the messages at this stage come from processing the GRPECS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but isn't run_start and run_end from RCT? Why GRPECS is used at all to retrieve the SOR?
print (f"INFO: GRPECS SOR ({SOR}) will be superseded by externally provided run start ({run_start})") | ||
SOR = run_start | ||
if run_end is not None: | ||
print (f"INFO: GRPECS EOR ({EOR}) will be superseded by externally provided run end ({run_end})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
run_start = run_duration.first | ||
run_end = run_duration.second | ||
|
||
GLOparams = retrieve_params_fromGRPECS(ccdbreader, args.run_number, run_start=run_start, run_end=run_end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it needed to pass run_start and run_end here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, to override what would otherwise be deduced from GRPECS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we simply not avoid to retrieve it from GRPECS now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get the orbit info from that. I asked @shahor02 specifically via email if we should stick to that method and as far as I understand, that should be the way. So we need to process GRPECS
for that reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but you can avoid retrieving SOR and EOR, this is what I mean. They will not be consistent with RCT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, that's fine with me. Let me update so that we will not use that info anymore. You are right, additional or somewhat redundant info should be omitted now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is what I meant.
I see you comments @chiarazampolli and mostly, they seem to go in the direction of "what's printed might be confusing". Maybe there could be some |
Hello @benedikt-voelkel , |
SOR=int(grp["mTimeStart"]) # in milliseconds | ||
EOR=int(grp["mTimeEnd"]) | ||
# cross check with RCT if available | ||
if rct != None: | ||
# verify that the variaous sor_eor information are the same | ||
if SOR != rct["SOR"]: | ||
print ("WARNING: Inconsistent SOR information on CCDB (divergence between GRPECS and RCT) ... will take RCT one") | ||
SOR=rct["SOR"] | ||
|
||
if EOR != rct["EOR"]: | ||
print ("WARNING: Inconsistent EOR information on CCDB (divergence between GRPECS and RCT) ... will take RCT one") | ||
EOR=rct["EOR"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took all these things out. Now, simply the given run_start
and run_end
are used where needed.
@chiarazampolli What do you think? Can we move on? |
Use BasicCCDBManager's getRunDuration to determine time range to be covered by simulation.
This will first try to get STF and ETF. If not available, the fallback chain is