One-Sided Match Targets

[Scienfitz]

Implements #632

NOT part of this PR: I just added a small admonition mentioning the new argument. Since this is a potentially confusing topic I would prefer to have a proper plot of one exemplary transformation for all 6 combinations of mismatch_instead and MatchMode. This can be another PR.

Normal Transformations (for reference)

One-Sided Match corresponding to <=1

One-Sided MisMatch corresponding to <=1

Amended doc on fork: https://scienfitz.github.io/baybe-dev/latest/userguide/targets.html#advanced-matching-options

[AdrianSosic]

I guess your approach does the job, but – if I'm not mistaken and my idea is correct – than it's overly complicated. Essentially, aren't we saying: whenever we do a one-sided match, we simply extend the function value to the corresponding side? For that, we already have the necessary machinery in place, but probably it didn't come to your mind since it has been used at the end of the transformation chain only: `ClampingTransformation". If you apply it the input, i.e. as the very first transformation, then you get exactly the desired result, no additional complicated logic needed, right? So this function here would become completely obsolete.

The way I'd implement its via some convenience method that can be called directly on an existing transformation (something like keep_value/extend_value – better name needed, but you know what I mean) and that only expects a reference point and a direction. And then the implementation in all match constructors would become trivial, namely you take the created object as-is and just call the new convenience method on it.

Or am I overlooking something?

[Scienfitz]

on the one hand yes, a corresponding ClampingTransformation can be equivalent to the TwoSidedAffineTransformation used here.

But I fail to see how it would change much of the logic, in particular how is that logic less complicated?

• You still have a helper. I can also shift the trf0 | trf step into the helper in the existing logic, is has not much to do with clamping or not
• You still need to decide between identity or the two respective clamping transformations, just like I now need to select between identity or the two respective two sided transformations

[AdrianSosic]

Perhaps I haven't explained properly. With your current code, you have:

• A private (and hence useless for the user) helper that sort of has the same role of clamping
• Because it includes the affine shift, you now had to adjust the logic of each match_ constructor, and they are now significantly harder to understand.

What I suggest: We add a .hold_output/hold_value/... method that only expects the "location" and the "direction" (the latter in terms of enum-compatible input). The consequence is:

• We have another nice reusable method for modifying existing transformation objects
• ALL match constructors keep their current easy-to-understand code and simply get an additional line:

  if match_mode is not `MatchMode.eq`:
      transformation = transformation.hold_output(match_value, match_mode)

  --> super easy to understand what is going on

At least as a rough sketch – details/namings to be adjusted

[Scienfitz]

Roughly what I imagined, and I still don't see that much gain but perhaps were getting there

what exactly is the role of match_value in hold_output? Keep in mind the clamping needs to be injected at the start, effectively holding one side of the input not the output. It just translates into the correct holding of the output at the right position if all shifts happen later. So If I want a simple injection utility, then I can only inject a clamp(min=0) or clamp(max=0). Injecting also the shift would violate your proposal that the existing constructors remain untouched (because they take care of shifts so far).

So tldr:

• cant see what I should do with match_value for the clamp. If I use it, everything largely remains the same as is now, except renaming
• If I just inject clamping and not shifting, its not according to your suggestion but would result in the more unaffected constructors

Or do you want to append the clamp? The situation cant arise currently, but I think thats not possible for non-monotonic transformations. We do not want to clamp the output in general, we want to clamp at a certain x position (appending clamp focuses on y not x)

[Scienfitz]

can you just lay out the envisioned steps of hold_output ?

[AdrianSosic]

I've just push a draft with a (rough) first sketch. The implication is that you get a really simple ._hold_output call appended to each match construction - no further logic needed. And both targets and transform have the corresponding convenience methods now, which is useful in its own right. Of course, we can still change how much of this we want to make available, decide about naming etc. What do you think?

One other thing that came to my mind: while it actually works out-of-the-box to specify both mismatch_instead AND a matching mode other than =, it sort of doesn't fully make sense to encounter such a situation. Because for symmetric match transformations, you could always just omit the mismatch_instead flag in this case and change the direction of the mode. So this perhaps would indicate a misconfiguration on the user side, and we might want to either disallow it or raise a warning? Or do you think there could be cases where transforms/targets are generated somehow programmatically and allowing this option could still be useful?

[AdrianSosic]

PS: note that I haven't touch/reviewed any other part of the code so far, was only looking at the match mode logic itself

[Scienfitz]

ok makes sense thx. I'm not sure about putting the target mach mode into utility -> makes no sense as its clearly target related -> targets.enum makes more sense imo

regarding the interplay of mismatch_instead and match_mode != "=": So your saying (mismatch_instead=True, match_mode=">=") is equivalent to (mismatch_instead=False, match_mode='<=')? Did you check this? I think there is a difference because of which side gets cuff off

1. >= + mismatch_instead=False: entire right region is equally good
2. >= + mismatch_instead=True: entire right region is equally bad
3. <= + mismatch_instead=False: entire left region is equally good
4. <= + mismatch_instead=True: entire left region is equally bad
   If I compare now 2 with 3, they are not the same conceptually or what am I missing? The spahe of the "other" region is decided by the transformation.

For bounded transforms it corresponds to an x shift:
(>=, False)
(<=, True)

For unbounded transforms its a completely different thing:
(>=, False)
(<=, True)

[AdrianSosic]

Yeah, nevermind, forget the comment about the interplay 🙈

But regarding placement of enum: there was a reason why I moved this to a generic place. You see, I removed all "target specific" context from the docstring, and now it is really a general-purpose enum concerning the matching of real-valued numbers. The reason why I did this is because the transformation subpackage needs it, and this package should be independent of the targets subpackage. When you keep the enum in targets, you'll notice this "design flaw" by the induced circular import that you now have to work around somehow. So if you ask me, I'd say let's rather move it to a central location, but still import it in the targets namespace, so that a user can still do from targets import MatchMode, NumericalTarget if they want to define their targets.

[Scienfitz]

imo youre trying too hard to abstract, the matching concept clearly belong to targets... which have literally construtors called match_.

The circular import is no problem via lazy import (which is done for many other parts of the code too), but here an alternative: the transformations don't really need _hold_output at all, matching doesn't exist as a concept for them - so why are targets not just calling hold_output_left_from or hold_output_right_from depending on the macth mode? MatchMode would then never leave the target namespace

[Copilot]

Pull Request Overview

This PR implements one-sided match targets by adding a match_mode argument to all match_* constructors in NumericalTarget, allowing users to specify whether matching should be exact (=) or one-sided (<= or >=).

• Introduces MatchMode enum with three values: le (<=), eq (=), and ge (>=)
• Adds match_mode parameter to all match_* constructors with default value MatchMode.eq
• Implements transformation compression for better performance when combining shift operations with bell, triangular, and two-sided affine transformations

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
baybe/targets/enum.py	Defines new MatchMode enum for different matching modes
baybe/targets/numerical.py	Adds match_mode parameter to match constructors and implements output holding logic
baybe/transformations/base.py	Adds methods for holding transformation output beyond certain values
baybe/utils/interval.py	Adds arithmetic operators for scalar addition/subtraction on intervals
baybe/transformations/utils.py	Enhances transformation compression to handle shift operations with other transformations
tests/test_targets.py	Updates tests to cover all match mode combinations
tests/test_transformations.py	Adds tests for transformation compression with shift operations
baybe/targets/init.py	Exports new MatchMode enum
docs/userguide/targets.md	Documents new one-sided matching functionality
CHANGELOG.md	Records the new feature

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[AdrianSosic]

the phrase entire number region is a bit ill-defined in this context. Just from the words, I could easily interpret this as the entire number line

[Scienfitz]

What I mean here is clearly the entire number region as specified by the operator but that last bit to me seemed unnecessary. Should I add it back?

[AdrianSosic]

or maybe shorter, how about: the entire associated interval (I think also number region is rather uncommon, no?)

[Scienfitz]

@AdrianSosic @AVHopp a proposal for the discussed picture and doc changes is up on fork