Add Switch node to correctionlib for flexible logic#318
Open
piepb42 wants to merge 3 commits intocms-nanoAOD:masterfrom
Open
Add Switch node to correctionlib for flexible logic#318piepb42 wants to merge 3 commits intocms-nanoAOD:masterfrom
piepb42 wants to merge 3 commits intocms-nanoAOD:masterfrom
Conversation
nsmith-
requested changes
Feb 9, 2026
Collaborator
nsmith-
left a comment
nsmith-
left a comment
There was a problem hiding this comment.
Thanks for implementing this!
| throw std::runtime_error("Switch node currently only supports numeric comparisons"); | ||
| } | ||
|
|
||
| sel.op = comp.getRequired<std::string_view>("op"); |
Collaborator
There was a problem hiding this comment.
Convert the string to an operation Enum here so that unknown operators raise on construction rather than evaluation. This also makes the comparison on evaluation much faster by not doing a string comparison for every call.
| else { | ||
| throw std::runtime_error("Unknown operator in Switch node: " + sel.op); | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
Once sel.op is an enum type, you can use a switch statement here
Comment on lines
+26
to
+48
| json_str = corr.model_dump_json(exclude_unset=True) | ||
| print(f"Generated JSON with Switch node:\n{json_str}\n") | ||
|
|
||
| cset_json = json.dumps({"schema_version": 2, "corrections": [json.loads(json_str)]}) | ||
| cset = correctionlib.CorrectionSet.from_string(cset_json) | ||
| evaluator = cset["boundary_test"] | ||
|
|
||
| print("-" * 40) | ||
| print("TESTING C++ EVALUATOR") | ||
| print("-" * 40) | ||
|
|
||
| val_inclusive = 3.0 | ||
| res_inclusive = evaluator.evaluate([val_inclusive]) | ||
| print(f"Input: {val_inclusive:<10} | Expected: 1.0 | Got: {res_inclusive}") | ||
|
|
||
| val_exclusive = 3.00001 | ||
| res_exclusive = evaluator.evaluate([val_exclusive]) | ||
| print(f"Input: {val_exclusive:<10} | Expected: 0.0 | Got: {res_exclusive}") | ||
|
|
||
| if res_inclusive == 1.0 and res_exclusive == 0.0: | ||
| print("\n[SUCCESS] The Switch node correctly handles inclusive boundaries!") | ||
| else: | ||
| print("\n[FAIL] Logic error in Switch node implementation.") |
Collaborator
There was a problem hiding this comment.
For the pytest framework to pick up this test you need to wrap this whole block into a test_switch() function.
Collaborator
|
@piepb42 any estimate when you can address the review comments? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressing issue: #310
This PR implements a new Switch node (schema v2). The standard Binning node does not support inclusive upper boundaries (e.g., 2.7 < eta <= 3.0), which are necessary for some CMS Jet ID corrections. The Switch node allows users to define a list of arbitrary comparisons (conditions) that are evaluated in order, similar to a Category node but with boolean logic.
Changes:
Documentation for using the JSON produced via switch node implementation and scripts for validation of this JSON vs Hard Coded Cuts: https://codimd.web.cern.ch/s/j8jmJ24HW#