-
Notifications
You must be signed in to change notification settings - Fork 154
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
enable parallel world in anchored simulation #1786
base: master
Are you sure you want to change the base?
Conversation
REQUEST FOR PRODUCTION RELEASES:
This will add The following labels are available |
@@ -312,6 +312,7 @@ def main(): | |||
parser.add_argument("-tf", type=int, help="number of timeframes per job", default=1) | |||
parser.add_argument("--ccdb-IRate", type=bool, help="whether to try fetching IRate from CCDB/CTP", default=True) | |||
parser.add_argument("--trig-eff", type=float, dest="trig_eff", help="Trigger eff needed for IR", default=-1.0) | |||
parser.add_argument("-enable-parallel-world", type=int, dest="enable_parallel_world", help="Enable parallel geometry", default=0) |
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 should not be needed. We can already pass simulation config key values from the outside.
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 for your review, I removed this in the latest commit
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.
In principle, we can pass config key values from the outside using the -confKey
of o2dpg_sim_workflow.py;
Before doing bigger changes such as in this PR, could you please check if this works or what are the show-stoppers with such approach?
@sawenzel thanks for your comment, I modified the implementation by forwarding the parallel world configuration to o2dpg_sim_workflow.py as you suggested. This gives the same results as before in the workflow creation |
@@ -216,6 +216,7 @@ baseargs="-tf ${NTIMEFRAMES} --split-id ${SPLITID} --prod-split ${PRODSPLIT} --c | |||
remainingargs="-seed ${SEED} -ns ${NSIGEVENTS} --include-local-qc --pregenCollContext" | |||
remainingargs="${remainingargs} -e ${ALIEN_JDL_SIMENGINE} -j ${NWORKERS}" | |||
remainingargs="${remainingargs} -productionTag ${ALIEN_JDL_LPMPRODUCTIONTAG:-alibi_anchorTest_tmp}" | |||
remainingargs="${remainingargs} -confKey \"GeometryManagerParam.useParallelWorld=${ENABLE_PARALLEL_WORLD};GeometryManagerParam.usePwGeoBVH=${ENABLE_PARALLEL_WORLD};GeometryManagerParam.usePwCaching=${ENABLE_PARALLEL_WORLD}\"" |
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 ideal. Someone might give -confKey
from the outside ... and this will override the external choice of parameters.
if [ ! "${CCDB_RC}" == "0" ]; then | ||
echo_error "Problem during CCDB prefetching of ${CCDBOBJECTS_IDEAL_MC}. Exiting." | ||
exit ${CCDB_RC} | ||
if [ "${ENABLE_PARALLEL_WORLD}" == "0" ]; then |
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.
could you please explain a bit what these changes are doing/achieving?
@mconcas @sawenzel I added the parallel world configurables in the python and bash scripts for the anchored workflow creation. I also added a variable in the pp test script to enable the parallel geometry navigation