Characterization and propagation of task type based on cmsDriver steps #11680
Conversation
|
@amaltaro This is still very green, but is this what you were referring to? I.e.: Get Then, what? Do we create a field: |
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
@amaltaro Here is the current logic I came up with:
Does this makes sense to you? Anything I'm missing? We still would need to validate the criteria to go from --steps arguments to the physics CMS task types. We would need input from Bugra and Sunil for that. |
|
Jenkins results:
|
|
Jenkins results:
|
There was a problem hiding this comment.
@khurtado Kenyi, I think you are in the right track here to discover and publish the step physics type, which involves:
- discovering the physics type in ReqMgr2. I am still unsure how to do this in the most efficient form, but ideally it should be done at the point that we load the PSet to do other things.
- provide getters/setters at WMTask layer (or maybe the best would be to have a WMStep layer, to cover multi step jobs).
- provide this new attribute at the job creation (arguments that get pickled)
- load the job pickle file and pass it over as a job classad
I know this is a work in progress, but on what concerns the naming convention, I think our best option right now would be to call it like physicsStepType or physicsTaskType in the source code.
Just an observation, some of the PSets come filled of secondary LFNs, making it fairly heavy to load and parse in memory. Given that the cmsDriver command always come in the beginning of the file, I wonder if we could parse only the head of the file.
Another thought that just occurred to me is, why can't we have cmsDriver command to provide us with a commented out line with the map of --steps to an actual physics type? This way we are no longer responsible for this logic, which honestly speaking, it's a hard spot to be, especially because we are not the origin of this information. What do you think?
|
@amaltaro Thank you! I like the physicsStepType or physicsTaskType names. I will change it to that. I will see how NOT to load the whole PSET but only read the first few lines instead. I do agree that if we could have the actual physics type as a commented out line in the pset, then we would not need to guess it from the |
|
Jenkins results:
|
|
Jenkins results:
|
|
please test this |
1 similar comment
|
please test this |
|
Jenkins results:
|
|
Jenkins results:
|
Yes, I fail to see any use case at the moment. But as you say, it could be that in the near future we find other usage for this physicsType at the CMSSW. And given that it's all already implemented, let us stick to these (unless unit tests proven to be too hard to make ;)) |
|
@amaltaro Please, check this PR when you get a chance. |
khurtado
force-pushed
the
10604
branch
2 times, most recently
from
4215dfa to
4f667e2
Compare
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
@amaltaro I fixed the merge conflicts, squashed the commits and added unit tests. This is ready for another PR review. |
|
Jenkins results:
|
khurtado
force-pushed
the
10604
branch
2 times, most recently
from
68acb7f to
1a0e788
Compare
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
|
@khurtado these changes look good to me. From what I can tell, all these changes should be backward compatible (such that UNKNOWN will be reported if a workload has been created before ReqMgr2 is running this code). Nonetheless, we need to be careful with this validation and validate both new and old workflows. FYI @todor-ivanov Kenyi, I updated the initial description to the best of my knowledge. However, please do pay more attention to PR and Issues description and make sure they are up-to-date. If you feel like the templates need improvement, I am happy to make those changes. |
Fixes #11712
Status
Ready for review
Description
Parse the job PSet configuration and extract relevant fields of the cmsDriver command to map them to a given physics type.
A detailed view of the changes provided in this PR is:
physicsTaskTypeattribute as well; later used by JobSubmitter and SimpleCondorPluginCMS_extendedJobTypecondor classad is provided, being a comma separated list of physics task typeIs it backward compatible (if not, which system it affects?)
NO
Related PRs
https://github.com/dmwm/cms-htcondor-es/pull/206/files
External dependencies / deployment changes
None