-
Notifications
You must be signed in to change notification settings - Fork 13
Add TopCP transformer query #566
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
Conversation
kyungeonchoi
commented
Mar 5, 2025
- TopCP transformer is a new transformer in ServiceX that runs the ATLAS TopCPToolkit framework
- This PR adds features to run TopCP transformer
- The ServiceX backend version 1.X.X is required to submit TopCP query
servicex/topcp/topcp.py
Outdated
| particleYaml = yaml.safe_load(particle_file) | ||
|
|
||
| query = { | ||
| "RecoYAML": recoYaml, |
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.
Q on this -
- maybe it would be easier just to call it "reco", "parton", "particle" ?
- I guess we should assume that if the user provides a YAML file they wish to run that particular type of analysis? so by default if someone provides a particle YAML then RunParticle should be True?
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.
- maybe it would be easier just to call it "reco", "parton", "particle" ?
I agree. Let me also change camel case field names to snake case.
- I guess we should assume that if the user provides a YAML file they wish to run that particular type of analysis? so by default if someone provides a particle YAML then RunParticle should be True?
I'm following the default values of TopCPToolkit except no_systematics. I'm open to change the defaults but I guess it's common to have all reco.yaml, parton.yaml and particle.yaml in the same directory and mostly run reco.yaml only.
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.
Oh? How does TopCP determine which one(s) to run?
It just seems to me that there is unlikely to be any situation where someone provides a particle-level YAML but doesn't expect the particle analysis to be run by default... ?
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.
That's fair - TopCP doesn't take each yaml input file. A user provides a directory which contains all yaml files instead. I think your proposal makes more sense as ServiceX query. Let me drop options like run_parton.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #566 +/- ##
==========================================
+ Coverage 96.09% 96.19% +0.10%
==========================================
Files 28 29 +1
Lines 1816 1865 +49
==========================================
+ Hits 1745 1794 +49
Misses 71 71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
* Remove obsolete fields in transform requests * Limit Sphinx versions due to 8.2.0 bug
* Limit simultaneous downloads; retry failed downloads * Better handling of fail_if_incomplete, access to set concurrency * Switch S3 client to boto3
* Update codecov execution
* Add free-thread Python 3.13 and preview Python 3.14 to test matrix * Setup python with uv? * No system uv
updates: - [github.com/pycqa/flake8: 7.1.2 → 7.2.0](PyCQA/flake8@7.1.2...7.2.0)
… handling (#579) * Use aioboto3's built-in concurrency handling * Add test coverage
* Support fail_on_missing_trees option
Bumps the actions group with 1 update: [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish). Updates `pypa/gh-action-pypi-publish` from 1.12.3 to 1.12.4 - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.12.3...v1.12.4) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]>
for more information, see https://pre-commit.ci
|
Let's goooo |