Skip to content

Add batch-wise cost estimation - #151

Closed
ll-nick wants to merge 1 commit into
mainfrom
batch-cost-estimation
Closed

Add batch-wise cost estimation#151
ll-nick wants to merge 1 commit into
mainfrom
batch-cost-estimation

Conversation

@ll-nick

@ll-nick ll-nick commented Jan 27, 2026

Copy link
Copy Markdown
Collaborator

Closes #150

This enables the cost estimator to score a batch of trajectories rather than processing them one by one.
This is opt-in, i.e. the latter is still possible.

#minor

@ll-nick ll-nick self-assigned this Jan 27, 2026
@ll-nick

ll-nick commented Jan 27, 2026

Copy link
Copy Markdown
Collaborator Author

@orzechow This is still very much a work in progress (it still needs some tidying up, tests and python bindings) but I wanted to discuss the interface with you before putting in too much work.

The other day I already started implementing an interface and it turned out to be harder than expected to come up with something nice that is backwards compatible.
0c20111 implements a first draft of what that could look like.

Instead of implementing the abstract CostEstimator, the user also has the option to implement the BatchCostEstimator. The latter gets a list of commands (and whether or not they are active) and returns a list of costs rather than a single double.It is still possible to use the CostEstimator as is. Under the hood, there is an adapter that allows the CostArbitrator to always work with the batch-wise type.

Now here's the issue that I wanted to discuss. The CostEstimator is a member of the option rather than the arbitrator.
In theory, this enables each option to use a different cost estimator.
With batch-wise cost estimation, that gets a little more difficult.
Right now, I solve this by first grouping all valid options by the cost estimator they use, then iterate over these groups to compute the scores.
That should work but I'm not sure this is actually what we want.

The implementation would be quite a bit simpler if instead the cost estimator would be part of the arbitrator.
It takes away some of the flexibility but is also a little more intuitive (at least to me).
I don't know when it would make sense to have different cost estimators living inside a single arbitrator.
Changing that, however, would be a breaking change since the cost estimator would be passed in the constructor of the CostArbitrator, rather than in the addOption method and we would simply pass all valid options to this single cost estimator.

Sorry, this turned out a bit longer than I had hoped, so here's a quick summary of the options:

  1. Do not implement this feature at all.
  2. CostEstimator stays a member of Option leading to a more complex implementation and probably a small runtime cost for grouping the options.
  3. CostEstimator move up a level into the arbitrator which simplifies the implementation but is a breaking change.

What do you think?

@ll-nick

ll-nick commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this one in favor of #152

@ll-nick ll-nick closed this Feb 9, 2026
@ll-nick
ll-nick deleted the batch-cost-estimation branch February 11, 2026 11:08
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.

Should cost estimator know about all options?

1 participant