feat: Experimental implementation of custom modifiers - #1991
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1991 +/- ##
==========================================
- Coverage 98.28% 98.09% -0.19%
==========================================
Files 69 70 +1
Lines 4538 4626 +88
Branches 803 814 +11
==========================================
+ Hits 4460 4538 +78
- Misses 45 51 +6
- Partials 33 37 +4
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:
|
|
do we want to differentiate between |
|
My reading of "contrib" is "quite stable API, but might get factored out eventually", while I would view "experimental" more like "anything goes, expect things to break". You could of course communicate how exactly you want the modules to be understood, but this is what I would think without further context. |
Yes, I think that's a good idea if we really mean "experimental". I have the same take as @alexander-held does. |
|
Ok ! |
9d8d660 to
789097d
Compare
|
Related to the discussion in scikit-hep/cabinetry#382, it may be good to provide a suitable schema for this as well (perhaps same PR makes sense?). |
|
If I may add a request, it would be great if |
789097d to
c758f7e
Compare
c758f7e to
668073b
Compare
|
Hi, just to comment also here that on the side of an ATLAS analysis there is some urgency with this and a merge would help us. |
668073b to
4bad866
Compare
6fab1ae to
4bf9844
Compare
4bf9844 to
b857d9d
Compare
|
Hi, Cheers |
1 similar comment
|
Hi, Cheers |
b857d9d to
55bc18f
Compare
55bc18f to
1d0600e
Compare
for more information, see https://pre-commit.ci
a640c0b to
10be48a
Compare
matthewfeickert
left a comment
There was a problem hiding this comment.
@kratsg I assume that these prints were put in here for debugging? and that we can remove them and they aren't actually supposed to be in there for logging. Can you confirm?
| self.builder_data[key][sample]["data"]["mask"] += moddata["mask"] | ||
| if thismod: | ||
| if thismod["name"] != func_name: | ||
| print(thismod) |
There was a problem hiding this comment.
| print(thismod) |
There was a problem hiding this comment.
I've removed the prints in bb302c2 but if we need them back for debugging quickly just rever it.
|
Relevant RTD build: https://pyhf--1991.org.readthedocs.build/en/1991/api.html#experimental |
|
Small edge case: if you define a custom parameter to plug into |
|
While giving this another try, I ran into something that is either a bug or a usage question. I also have some API feedback. I opened #2350 for this to not clutter this PR too much. |
Pull Request Description
Refer to #850.
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees: