Skip to content

Translate weights in sum mode - #959

Draft
thehrh wants to merge 36 commits into
masterfrom
translate_weights_in_sum_mode
Draft

Translate weights in sum mode#959
thehrh wants to merge 36 commits into
masterfrom
translate_weights_in_sum_mode

Conversation

@thehrh

@thehrh thehrh commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Meant to complete what #934 began: probability weights shouldn't transform in "average" mode by default, but in "sum" mode.

  • this implies that under a transformation to a histogram/map representation, the Container class will sum—and not average—weights
  • the inverse ("unbinning") or changing the binning ("rebinning") is not generally possible without making approximations and is thus not implemented (this is currently the case in PISA and isn't changed by this PR)
  • in "average" mode, however, PISA performs a) a simple bin-value lookup for unbinning and b) resampling for rebinning, which is problematic for our probability weights due to the resulting change in normalisation (verify this by transforming a weight event-count histogram (back) to "events" representation or by merging bins)

Changes

  • add "weights" and "initial_weights" keys to so-far empty Container.sum_mode_keys attribute
  • change hierarchy of if statements in Container.translate(), checking for translation route—not mode— first, and
    • no longer distinguish between "average" and "sum" modes in case of one-to-one relationship, i.e., for "events" ↔ "log_events" (distinction only relevant when aggregating/unbinning/rebinning; NB: before, for undocumented reasons, these routes were rejected for "sum" mode)
  • additionally, supply method Container.set_item_no_invalidate(), which doesn't invalidate any representations
  • expand test_container() unit tests to demonstrate expected behaviour and ensure sanity
  • adapt test_Pipeline() unit tests because "weights" cannot be translated from binned to "events" output any longer
  • major rewrite of utils.hist's setup_function for the (unconventional) case of a binned calc_mode
    • TODO
  • slightly expand and fix pisa_modes.ipynb introductory text on representations/modes and gently restructure cells to execute
  • expand logging and documentation
  • also fix a spelling mistake in run_unit_tests.py

thehrh and others added 30 commits April 24, 2026 18:54
…longer; add docstring note about absence of setup+compute/apply instead
…nding function isn't implemented (for now, possibly change later) [no ci]
…d include md output in stage modes notebook for now (TODO: fix notebook) [no ci]
…tion_modes -> translation_modes; representation management unit test; update container module docs
….hist assertion fails otherwise, and adapt pipeline unit test temporarily
… document all __getitem__ and __setitem__ methods throughout code base [no ci]
… rewrite utils.hist (with calc_mode=binning) accordingly; also give Container a convenient item setter that doesn't invalidate any reps. and extend tests
thehrh added 5 commits June 24, 2026 19:20
…conflicts; adapt pipeline unit test to reflect new treatment of weights; finally wile -> while
…iable, add comments, and integrate additional sanity checks into translate()
…utating Container data with or without invalidating other reps
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.

1 participant