Enable Coefficients for `DiscreteSumConstraint` and `from_simplex` by Scienfitz · Pull Request #786 · emdgroup/baybe

Scienfitz · 2026-04-29T15:39:26Z

use-case motivated addition:

DiscreteSumConstraint gets a coefficients that works akin to whats been done in the continuous constraint
from_simplex gets a simplex_coefficients keyword that allows specifying coefficients for the simplex parameters. This is possible by changing the way the max/min incoming sums are assessed
I'm using matrix multiplication @ for from_simplex because we ensure by construciton that the incoming array is contiguous and does not have to be copied for a reshape and multiplication. This contiguousness is not guaranteed for data[params] in get_invalid in DiscreteSumConstraint so its more memory efficient to use per-column approaches in the assumption of a small countable amount of parameters (usually the case)

Unrelated optimization
I also replaced the inner loop of from_simplex to use numpy and not pandas. This is also motivated by memory and time efficiency. This commit can be dropped tho if undesired, but there are singificant time and mem savings:

Scenario	Rows	main time (s)	feature time (s)	Δ time	main mem (MB)	feature mem (MB)	Δ mem
4p × 11v	1,001	0.013	0.002	-88%	0.4	0.2	-49%
6p × 11v	8,008	0.039	0.002	-94%	4.3	3.2	-26%
8p × 11v	43,758	0.193	0.011	-94%	35.1	27.9	-20%
6p × 21v	230,230	0.672	0.032	-95%	141.1	106.1	-25%
6p × 21v boundary	53,130	0.736	0.032	-96%	141.1	106.1	-25%

(p = simplex parameters, v = values)

Copilot

Pull request overview

This PR extends BayBE’s discrete constraint and search space construction capabilities by adding weighted-sum support to DiscreteSumConstraint and enabling weighted simplex construction via SubspaceDiscrete.from_simplex(simplex_coefficients=...). It also refactors the hot loop in from_simplex to use NumPy arrays for improved performance and memory usage.

Changes:

Add coefficients to DiscreteSumConstraint (defaulting to all ones) and apply weighting in both pandas and polars evaluation paths.
Add keyword-only simplex_coefficients to SubspaceDiscrete.from_simplex (defaulting to all ones) and adjust pruning logic to work with weighted sums.
Expand test coverage for the new weighted behaviors and length-mismatch validation.

Reviewed changes

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

Show a summary per file

File	Description
`baybe/constraints/discrete.py`	Adds `DiscreteSumConstraint.coefficients` with validation + weighted evaluation (pandas/polars).
`baybe/searchspace/discrete.py`	Adds `simplex_coefficients`, makes args keyword-only, and rewrites `from_simplex` construction loop using NumPy with weighted pruning.
`CHANGELOG.md`	Documents the new weighted features and the keyword-only breaking change for `from_simplex`.
`tests/validation/test_constraint_validation.py`	Adds validation test for `DiscreteSumConstraint` coefficients length mismatch.
`tests/hypothesis_strategies/constraints.py`	Updates Hypothesis discrete-constraint strategy generation to optionally include coefficients.
`tests/hypothesis_strategies/alternative_creation/test_searchspace.py`	Adds brute-force parity tests for weighted simplex generation and mismatch validation.
`tests/constraints/test_constraints_polars.py`	Adds parity test to ensure polars vs pandas agree for weighted sum constraints.
`tests/constraints/test_constraints_discrete.py`	Adds behavioral tests for weighted sum constraints in the discrete constraint suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

AVHopp

Main question is whether or not we still need the assumption on the non-negativity of parameter values

Follows the ContinuousLinearConstraint pattern: coefficients default to all-ones (preserving existing behavior), are validated for length parity with parameters, and the weighted sum is evaluated via a single numpy matrix-vector product to avoid intermediate DataFrame copies.

Reworks the signature to make all optional arguments keyword-only (via *). Adds simplex_coefficients for a weighted simplex sum constraint. The incremental early-pruning algorithm is generalised to handle negative coefficients correctly by computing per-parameter weighted min/max contributions rather than assuming monotonicity, and by keeping nonzero cardinality tracking separate (raw parameter values, coefficient-sign independent). The weighted row-sum uses a single numpy matrix-vector product to avoid intermediate DataFrame copies.

…plex_coefficients Weighted-sum filtering correctness (default and custom coefficients) added to the existing discrete constraint test file, parametrized across all-ones, scaled, negative, and equality operator cases. Simplex coefficient tests (brute-force equivalence, mixed-sign, boundary_only, and equivalence with from_product+DiscreteSumConstraint) added to the existing from_simplex test file. Validation error tests for length mismatch added to the constraint validation test file.

… sum The previous approach (to_numpy() @ np.asarray(coefficients)) consolidates all referenced columns into a contiguous (N, k) array before computing the dot product. When the constraint parameters are non-adjacent columns in the DataFrame this forces a full (N, k) memory copy regardless. For the typical use case of sum constraints (k < 10 parameters), a column-by-column accumulation avoids this: each data[p].to_numpy() is a zero-copy view of a single contiguous column, the scalar multiply produces one (N,) temporary, and the built-in sum accumulates in-place. No (N, k) consolidation allocation is needed. Also removes the now-unused numpy import.

Replaces the pandas-based inner loop (pd.merge cross-join, pd.DataFrame, df.drop inplace) with raw numpy operations (np.repeat + np.tile + np.column_stack for cross-joins, boolean indexing for pruning). The DataFrame is created once at the end. This avoids per-iteration pandas overhead (index management, BlockManager, merge machinery) and reduces peak memory by eliminating duplicate DataFrame+numpy representations.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

AdrianSosic · 2026-06-08T13:02:57Z

+        evaluate_df = pd.Series(
+            sum(
+                df[p].to_numpy() * c for p, c in zip(self.parameters, self.coefficients)
+            ),
+            index=df.index,
+        )


Suggested change

evaluate_df = pd.Series(

sum(

df[p].to_numpy() * c for p, c in zip(self.parameters, self.coefficients)

),

index=df.index,

)

evaluate_df = df[self.parameters] @ self.coefficients

did you see this comment in the PR description

i am prioritizing not doing copy operations here at the cost of having to do several computations instead of one big vectorized one. in the limit of few parameters (generally the case for us) this should be the better choice

AdrianSosic · 2026-06-08T13:22:39Z

+                simplex_coefficients = converter.structure(
+                    simplex_coefficients, list[float]
+                )
+            except (IterableValidationError, TypeError, ValueError) as exc:


I'm already looking forward to the point where all of this suddenly becomes obsolete with the new lazy framework 🦆 RE exception: why such a strict collection of exception types and not a pragmatic Exception instead? Otherwise you'd need to be 100% certain to catch all cases, or are you expecting other exceptions that could happen in that single statement?

this is all just attrs related, not even sure how its related to the refactoring but turning it into a broad Exception is not an improvement

form my perspective these are the only exceptions that can happen if an invalid function input is provided, so what exactly do you want improved?

only alternative I see is maybe not even reraising, hence getting rid of the except but perhaps having a worse error message

AdrianSosic · 2026-06-08T13:30:52Z

                np.append(max_nonzero_upcoming, 0),
            )
        ):
+            values = np.asarray(param.values, dtype=float)


Since #803, I've mostly rather seen to_numpy calls on the dataframe instead. Is this one here intended/compatible?

i think this PR is not rebased on that yet and lines like this prob need to be fixed

after looking at this line I think tis completely unrelated

this lines calls asarray on a tuple (param.values) which always creates a new array, hecn eno problem possible downstream anywhere

AdrianSosic · 2026-06-08T13:58:29Z

-                    exp_rep, pd.DataFrame({param.name: param.values}), how="cross"
+                n_old = arr.shape[0]
+                n_new = len(values)
+                arr = np.column_stack(


Happy about the numpy-change, but since we're now talking about speed/memory optimizations, let's directly go berserk mode: the current stacking approach is still suboptimal (I think) because it materializes the intermediate arrays. Instead, we should go via broadcasting. I've run a very brief test that promises even further improvements! Can you take care of it?

i can give it a try but it would help if you could already point out what lines or what kind of lines would be affected in principle by the further improvements

AdrianSosic · 2026-06-08T13:59:46Z

+        param((1.0, 1.0), 1.0, "=", 6, id="equality"),
+    ],
+)
+def test_sum_constraint_coefficients(coefficients, threshold, operator, n_invalid):


Just for my understanding: does this mean that there was no test for DiscreteSumConstraint previously?

since the coefficients feature is new in this PR there was indeed no previous test for sum coefficients
(the constraint itself was tested in various places)

AdrianSosic · 2026-06-08T14:02:08Z

+        param((0.5, 0.5), 50.0, "=", id="weighted-eq"),
+    ],
+)
+@pytest.mark.parametrize("parameter_names", [["Fraction_1", "Fraction_2"]])


Do we really need a separate test here? Can't we generalize one of the already existing ones to take into account the coefficients (potentially taking this as an opportunity to clean up some of the redundant code instead of adding even more)?

consolidated some of the polars tests here d2c7aa3
so prodsum1 and prodsum3 now go together with the new test added in this PD, prodsum2 was renamed as it tests the product constraint

Im not really looking to refactor all the other tests in this file as they still use the fixture system we have started to move away from to be consisztent witht he non-polars tests which would then also need to change

AdrianSosic · 2026-06-08T14:06:52Z

+        )
+
+
+def test_continuous_linear_constraint_zero_coefficient():


shouldn't this test the same two conditions like the test above?

now done and consolidated 4f43355

AdrianSosic · 2026-06-08T14:18:39Z

+    assert_frame_equal(result, expected, check_dtype=False)
+
+
+def test_discrete_space_creation_from_simplex_coefficients_vs_from_product():


like the test above, this is yet another equivalence test, though without parametrization. Why not merge these two tests and simply assert that brute_force == simplex == product in all cases?

seems reasonable 0163e3c

AdrianSosic · 2026-06-10T11:22:45Z

-        min_values = [min(p.values) for p in simplex_parameters]
-        max_values = [max(p.values) for p in simplex_parameters]
-        if not (min(min_values) >= 0.0):
+        # Validate non-negativity of raw parameter values (required by the algorithm)


The potential issue I asked about in the other thread: my original construction was based on the assumption that parameter contributions are combined only additively! This was used to early-drop invalid combinations since you then know that once surpassed the limits, any additionally included parameter can only make the situation even worse, i.e. push you even further off the limits. Now that you allow possible negative coefficients, the contributions of the individual parameters to the total sum may also be be negative, which breaks the original logic. So what I'm asking: have you checked / how to you ensure that the early-discard-logic is still intact?

iirc the key element of the previous logic was not that upcoming sum contributions can only increase the sum

the key logic is that the row is doomed if even with the smallest upcoming contribution the sum is already bad (too large etc).

There is nothing in this that requires that an upcoming contribution can be only positive, right? So all this PR did was to correctly identify that minimial upcoming contrib - which now can also be negative due to coeffs

I dont think thats wrong, but in general it will of course lead to much less efficient consturcitons because a negative coeff will lead to less rows that are early dropouts

test_discrete_space_creation_from_simplex_coefficients currecntly tests one case of neg coefficients

I also think that the logic is correct here, and that this also enables us to allow negative values as well: We only care about contributions, those can be negative or positive. We do not care about the reason for the sign, which could be any combination of positive/negative coefficient with positive/negative value, right? So if we agree that the logic works for negative and positive contributions, then we should be able to drop the assumption on non-negativity for the values as well, right?

Scienfitz self-assigned this Apr 29, 2026

Scienfitz added enhancement Expand / change existing functionality new feature New functionality labels Apr 29, 2026

Scienfitz force-pushed the feature/sum_constraint_coefficients branch from f8b3f17 to cce898a Compare May 7, 2026 11:20

Scienfitz marked this pull request as ready for review May 7, 2026 11:48

Scienfitz requested review from AVHopp and AdrianSosic as code owners May 7, 2026 11:48

Copilot AI review requested due to automatic review settings May 7, 2026 11:48

Copilot started reviewing on behalf of Scienfitz May 7, 2026 11:49 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread baybe/searchspace/discrete.py

Comment thread baybe/searchspace/discrete.py Outdated

Scienfitz force-pushed the feature/sum_constraint_coefficients branch from 466cf21 to a232085 Compare May 7, 2026 12:29

AVHopp reviewed May 27, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread CHANGELOG.md

Comment thread baybe/searchspace/discrete.py

Comment thread baybe/searchspace/discrete.py

Comment thread baybe/searchspace/discrete.py

Comment thread baybe/searchspace/discrete.py

Scienfitz added this to the 0.16.0 milestone Jun 2, 2026

Scienfitz and others added 9 commits June 3, 2026 17:57

Use any() for non-negativity check in from_simplex

89deeab

Fix mypy error

a4af9af

Improve validation in from_simplex

43b9c99

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Improve deserialization validation

fa78764

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Scienfitz force-pushed the feature/sum_constraint_coefficients branch from a232085 to 3cc4c94 Compare June 3, 2026 16:03

Scienfitz added 4 commits June 3, 2026 18:28

Forbid all-zero coefficients in linear/sum constraints

1c9d4a7

Forbid individual zero coefficients in linear/sum constraints

583ba2b

Add tests for non-zero coefficient validation

d96d428

Update CHANGELOG

df93b36

Scienfitz force-pushed the feature/sum_constraint_coefficients branch from 3cc4c94 to df93b36 Compare June 3, 2026 16:56

AdrianSosic requested changes Jun 8, 2026

View reviewed changes

AVHopp reviewed Jun 9, 2026

View reviewed changes

Comment thread baybe/searchspace/discrete.py

AdrianSosic reviewed Jun 10, 2026

View reviewed changes

Scienfitz added 4 commits June 10, 2026 19:45

Unify coefficient validation tests for both constraint types

4f43355

Merge simplex coefficient tests into three-way comparison

0163e3c

Consolidate polars sum constraint tests

d2c7aa3

Improve docstring

2ce4a29

Scienfitz force-pushed the feature/sum_constraint_coefficients branch from d9f4faa to 2ce4a29 Compare June 10, 2026 17:51

		assert_frame_equal(result, expected, check_dtype=False)


		def test_discrete_space_creation_from_simplex_coefficients_vs_from_product():

Conversation

Scienfitz commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

AVHopp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scienfitz Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Scienfitz commented Apr 29, 2026 •

edited

Loading

Scienfitz Jun 10, 2026 •

edited

Loading