Skip to content

feat: add replaceValue... methods to MutableSolutionView#2180

Open
triceo wants to merge 6 commits intoTimefoldAI:mainfrom
triceo:smarter
Open

feat: add replaceValue... methods to MutableSolutionView#2180
triceo wants to merge 6 commits intoTimefoldAI:mainfrom
triceo:smarter

Conversation

@triceo
Copy link
Collaborator

@triceo triceo commented Mar 10, 2026

In addition to moveValue...

Copilot AI review requested due to automatic review settings March 10, 2026 19:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes moving and swapping elements inside planning list variables in MoveDirector, aiming to reduce per-move overhead on the solver hot path.

Changes:

  • Introduces a moveInList(...) helper that selects between Collections.rotate and remove+add based on move distance/position.
  • Updates moveValueInList(...) to use the new helper and return the moved value via the list after mutation.
  • Replaces manual swap logic with Collections.swap(...) in swapValuesInList(...).

Copy link
Contributor

@Christopher-Chianelli Christopher-Chianelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM when tests are green.

@triceo
Copy link
Collaborator Author

triceo commented Mar 11, 2026

@Christopher-Chianelli Since I was already here, I decided to implement a feature request from the models. Please re-review, new code was added.

@winklerm Was this what you had in mind?

@triceo triceo linked an issue Mar 11, 2026 that may be closed by this pull request
@triceo triceo added this to the v2.0.0 milestone Mar 11, 2026
@triceo triceo requested a review from Copilot March 11, 2026 08:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings March 11, 2026 10:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings March 11, 2026 10:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

…tableSolutionView.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 11, 2026 10:36
@triceo triceo deployed to internal March 11, 2026 10:36 — with GitHub Actions Active
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@triceo triceo changed the title perf: improve value move within a list feat: add replaceValue... methods to MutableSolutionView Mar 11, 2026
@sonarqubecloud
Copy link

* while the sourceEntity's list size remains unchanged.
*
* @param variableMetaModel Describes the variable to be changed.
* @param sourceEntity The entity in which the value at {@code sourceIndex} will be replaced (overwritten).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: I would expect destinationEntity and destinationIndex instead of sourceEntity and sourceIndex, but might be just my personal preference.

@winklerm
Copy link
Contributor

@winklerm Was this what you had in mind?

Yes, thank you very much!!

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.

Feat: MutableSolutionView.replaceValue()

4 participants