Fix two Neighborhoods bugs#2314
Open
triceo wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses two independent defects by (1) fixing neighborhood stream iteration state handling to avoid persisting invalid iterators across solver steps, and (2) changing multiple config classes to store class references as class-name strings (resolved lazily) so config I/O remains stable and inheritance works on raw configured values.
Changes:
- Removed tuple-store-based caching of right-side iterators and replaced it with iterator-local caching in
BiRandomMoveIterator. - Simplified left dataset instantiation by removing the now-unused “right iterator store index” plumbing.
- Introduced
ConfigUtils.resolveClass(...)and updated many config classes to store class names (String) instead ofClass<?>, resolving on demand in getters/visitors.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/uni/UniLeftDatasetInstance.java | Updates constructor/signature to match removal of right-iterator store index. |
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/uni/UniLeftDataset.java | Updates dataset instantiation API to only reserve/pass entry store index. |
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/uni/LeftTerminalUniEnumeratingStream.java | Reserves a single tuple store index for left dataset instance creation. |
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/common/AbstractLeftDatasetInstance.java | Removes right-iterator store index field/accessor. |
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/common/AbstractLeftDataset.java | Updates abstract instantiate signature to reflect removed index. |
| core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/BiRandomMoveIterator.java | Fixes iterator caching scope by storing right iterators in a local map instead of tuple store. |
| core/src/main/java/ai/timefold/solver/core/config/util/ConfigUtils.java | Adds resolveClass(...) helper for lazy class resolution from stored class-name strings. |
| core/src/main/java/ai/timefold/solver/core/config/solver/SolverManagerConfig.java | Stores threadFactoryClass as string; resolves lazily in getter/visitor/inheritance. |
| core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java | Stores threadFactoryClass, solutionClass, entityClassList, nearbyDistanceMeterClass as strings; resolves lazily and inherits raw values. |
| core/src/main/java/ai/timefold/solver/core/config/score/director/ScoreDirectorFactoryConfig.java | Stores calculator/provider classes as strings; resolves lazily and inherits raw values. |
| core/src/main/java/ai/timefold/solver/core/config/phase/custom/CustomPhaseConfig.java | Stores custom phase command classes as strings; resolves lazily and inherits raw values. |
| core/src/main/java/ai/timefold/solver/core/config/partitionedsearch/PartitionedSearchPhaseConfig.java | Stores partitioner class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/localsearch/LocalSearchPhaseConfig.java | Stores neighborhood provider class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/value/ValueSelectorConfig.java | Stores several selector-related classes as strings; resolves lazily and inherits raw values. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/move/generic/MultistageMoveSelectorConfig.java | Stores stage provider/entity classes as strings; resolves lazily and inherits raw values. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/move/generic/list/ListMultistageMoveSelectorConfig.java | Stores stage provider class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/move/factory/MoveListFactoryConfig.java | Stores move list factory class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/move/factory/MoveIteratorFactoryConfig.java | Stores move iterator factory class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/move/composite/UnionMoveSelectorConfig.java | Stores probability weight factory class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/entity/EntitySelectorConfig.java | Stores selector-related classes as strings; resolves lazily (but toString() now resolves too). |
| core/src/main/java/ai/timefold/solver/core/config/heuristic/selector/common/nearby/NearbySelectionConfig.java | Stores nearby distance meter class as string; resolves lazily and inherits raw value. |
| core/src/main/java/ai/timefold/solver/core/config/constructionheuristic/placer/QueuedValuePlacerConfig.java | Stores entity class as string; resolves lazily and inherits raw value. |
Comment on lines
342
to
345
| @Override | ||
| public String toString() { | ||
| return getClass().getSimpleName() + "(" + entityClass + ")"; | ||
| return getClass().getSimpleName() + "(" + getEntityClass() + ")"; | ||
| } |
Comment on lines
+58
to
+65
| var tccl = Thread.currentThread().getContextClassLoader(); | ||
| var cl = tccl != null ? tccl : ConfigUtils.class.getClassLoader(); | ||
| try { | ||
| return (Class<T>) Class.forName(trimmedClassName, true, cl); | ||
| } catch (ClassNotFoundException e) { | ||
| throw new IllegalArgumentException( | ||
| "The %s (%s) of %s cannot be found.".formatted(fieldName, trimmedClassName, context), e); | ||
| } |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See attached issue for a description of what was wrong.
Independent bugs, independent commits. Will not be squashed.