Skip to content

Patch setupRabbit.py operations#671

Open
lucalavezzo wants to merge 3 commits intoWMass:mainfrom
lucalavezzo:luca-dev
Open

Patch setupRabbit.py operations#671
lucalavezzo wants to merge 3 commits intoWMass:mainfrom
lucalavezzo:luca-dev

Conversation

@lucalavezzo
Copy link
Collaborator

No description provided.

@cippy
Copy link
Collaborator

cippy commented Mar 12, 2026

As I commented in Mattermost, I think this change of --select that adds the sum keyword to keep or remove the axis after the operation is making --select equivalent to --axlim (at least in the default case), which slices but doesn't remove the axis.
If the issue you found is that with --fitvar x-y and --select z A B the selection on z was ineffective because the histograms were projected onto x-y summing the full z axis before it could be selected, then the issue is that we don't have an option to specify when an action has to take place before others.
In fact, in that case the correct usage should have been to call --fitvar x-y-z and then --select z A B, which would remove the axis (otherwise if you need to keep the axis we already have --axlim to do the very same and we don't need to modify the other existing options)

logger.debug("Applying global action")
h = self.globalAction(h)

sum_axes = [x for x in self.sum_gen_axes if x in h.axes.name]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are touching this part, can we take this occasion to rename sum_axes (which is only used in this and the next few lines) as sum_gen_axes or something to explicitly mark it as such? It is probably already quite clear from the right had side that it only affects gen axes, usually for the unfolding or alphaS, but if you did it would make this part of the code easier to understand. Other possible axes on which we might apply actions are managed elsewhere, for example through the globalAction.

sel_ax, sel_lb, sel_ub = sel.split()
parts = sel.split()
sel_ax, sel_lb, sel_ub = parts[0], parts[1], parts[2]
do_sum = len(parts) > 3 and parts[3] == "sum"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In spite of my previous comment, I think it makes sense that you made this action act more generically, allowing the user to control whether the marginalisation of the axis should happen or not.
Unfortunately it modifies the previous default behaviour, but if this is properly advertised to all users I can accept it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants