Skip to content
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

[FEATURE] .viz.pulls for normfactors #341

Open
andrzejnovak opened this issue Jun 8, 2022 · 4 comments
Open

[FEATURE] .viz.pulls for normfactors #341

andrzejnovak opened this issue Jun 8, 2022 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@andrzejnovak
Copy link
Member

.visualize.pulls Should only show the value (maybe just numeric) for unconstrained params

@alexander-held alexander-held added enhancement New feature or request good first issue Good for newcomers labels Jun 8, 2022
@alexander-held
Copy link
Member

Thanks for raising this! Unconstrained parameters do not fit very well into this plot. I like the idea of just showing the best-fit value and uncertainty as text, alternatively we could filter out such parameters automatically. Currently this can be done manually via the exclude kwarg, but that is tedious.

Once conceptual difficulty is that the function currently does not have access to the information about which parameter is unconstrained. We could either add that to the FitResults container (might be convenient to have for other purposes as well), or hand over the model (or just the relevant piece of information) in the pulls() function call.

Another thing to consider would be staterror parameters, they do not fit particularly well here either since they have a different central value and varying constraint term widths.

@andrzejnovak
Copy link
Member Author

Hah indeed, I was using exclude for staterror, but still wanted an easy way to "see" the interesting params. Indeed adding rgx/wildcards parsing to exclude would be one improvement and maybe exclude_by_type to do so by type (perhaps defaulting to ['staterror', 'normfactor']

@alexander-held
Copy link
Member

There currently is a hardcoded exclusion for parameters called staterror_*:

# exclude staterror parameters from pull plot (they are centered at 1)
exclude_set.update([label for label in labels_np if label[0:10] == "staterror_"])

which is not very elegant and will fail if users call them differently (which they are free to do).

I think it would be great to do both, as you suggest:

  • support exclusion by type with sensible defaults (for which we should probably save the types as metadata in FitResults),
  • more flexibility in exclusion via wildcards or regex.

We're currently using fnmatch to support wildcards in other places such as

region_matches = fnmatch.fnmatch(region_name, processor["region"])
which probably has a lower barrier to entry but is also less powerful than regex. Either could work here I think.

@vaustrup
Copy link

vaustrup commented Nov 22, 2023

Hi, just stumbled across this issue. I have an implementation for the exclusion by parameter type in my fork of cabinetry. If this is still of interest, I could open a PR?

Edit: Ah, nevermind, I see it being discussed in #342 already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants