chore: improve sorting operation

[zepfred]

This PR enables sorting the value ranges at the ValueRangeManager level.

The sorting feature is likely useful during the CH phase, and the new approach can address the sorting requirement for the existing CH configurations. However, the existing selector sorting nodes remain available, as they are required for range filtering or nearby selectors. Also, these corner cases may only make sense for the LS phases.

[zepfred]

@triceo The PR is ready for review and will rerun all previous model validations, just like we did when adding the sorting feature.

[zepfred]

There is an issue with the quickstart compilation job, and I will take a look.

[triceo]

I am not excited about how sorting creates yet another completely separate path for some selectors, and another part for others.

ValueRangeManager needs to be the ultimate source of truth. Think of the future, where we are inevitably going to be asked to enable mutable value ranges on entities. (When variables change, so do value ranges.) This complexity simply cannot be managed, if value ranges are all over the place. This needs to be centralized.
This future is closer than you think; early next year I expect.

IMO we should start cutting off the paths that lead to these discrepancies. Deprecating, failing fast. Especially when we introduce new functionality, like here - with new functionality, there is no backwards compatibility. We can simply decide to make some things impossible. We can start to reduce the impossible cross-combinations of move selector configurations.

Also, you are far too attached to the ValueRange type. In the future, this type will not exist. Every ValueRange will just be a Collection. Make your future you's life easier, and stop treating the ValueRange interface as a building block for everything.

We only have a limited amount of time to spend on this, and there are good things in here. Let's discuss where we go from here.

[triceo]

So... let's assume I have provided the nearby distance meter, and nothing else. (No selector configuration.) Does nearby use value ranges?

If not, then I wonder why we're really doing this. The idea of value ranges is that there would be one place where all of this happens, but if something as critical as nearby still ignores value ranges, then arguably the idea failed.

[zepfred]

So... let's assume I have provided the nearby distance meter, and nothing else. (No selector configuration.) Does nearby use value ranges?

I'm unsure if this is even possible. The point is, if there is a nearby configuration, then it means we will create nodes that ignore the sorting provided by the child selectors.

If not, then I wonder why we're really doing this. The idea of value ranges is that there would be one place where all of this happens, but if something as critical as nearby still ignores value ranges, then arguably the idea failed.

I see your point and will work on a solution. That means we will need to pass the sorter to the range filtering and nearby nodes.

[triceo]

There is an issue with the quickstart compilation job, and I will take a look.

The Vert.x exception is a red herring. Your real problem is here:

025-11-04 07:29:04,008 ERROR [org.acm.pro.res.ProjectJobSchedulingResource] (pool-8-thread-1) Failed solving jobId (52a73dc5-6458-4eda-bb33-61a1b2a59fd4).: java.lang.UnsupportedOperationException
at ai.timefold.solver.core.impl.domain.valuerange.AbstractCountableValueRange.sort(AbstractCountableValueRange.java:44)

[triceo]

I'm thinking we're approaching this all wrong.

We're building this hierarchy of selectors, decorating with additional behaviors. But value ranges shouldn't work like that. Value ranges are meant to be the root selector. When sorting is necessary, it should be applied to the root selector directly, not via some decorator later. The root iteration should already happen in the sorted order.

How do values come into the value range selector? Who puts them there? That is where we need to start - and that source of data needs to be the value range manager. Which will already have everything sorted. And then we can start building all the other functionality on top of that - the filter decorator etc., all of that can be layered on top of what's already sorted.

This is a large refactor, and maybe we don't want to do it right now. But this is the way to make sure that value range manager is the only source of truth for data. And consequently, it is the only way how we can safely produce value ranges which change with their entity variables.

[zepfred]

I'm thinking we're approaching this all wrong.

We're building this hierarchy of selectors, decorating with additional behaviors. But value ranges shouldn't work like that. Value ranges are meant to be the root selector. When sorting is necessary, it should be applied to the root selector directly, not via some decorator later. The root iteration should already happen in the sorted order.

That's the current approach. The base method buildBaseValueSelector creates the root node, which uses the sorter and fetches the sorted value range when needed. I'm currently working to remove the step applySorting when filtering range nodes or nearby selectors are used.

How do values come into the value range selector? Who puts them there? That is where we need to start - and that source of data needs to be the value range manager. Which will already have everything sorted. And then we can start building all the other functionality on top of that - the filter decorator etc., all of that can be layered on top of what's already sorted.

Yes, that's correct. However, the range filtering and the nearby nodes ignore the root selector sort order because they use different structures to fetch the values: reachable values and the distance matrix.

This is a large refactor, and maybe we don't want to do it right now. But this is the way to make sure that value range manager is the only source of truth for data. And consequently, it is the only way how we can safely produce value ranges which change with their entity variables.

I have a solution to maintain the ValueRangeManager as the single source of truth for the reachable values structure and have not yet looked into the nearby selector.

[zepfred]

@triceo I'll begin reviewing the necessary changes to the enterprise repository, and ValueSelectorFactory will also be modified to pass the sorter to the inner nodes.

This change is needed because both FilteringValueRangeSelector and the nearby nodes use a different structure to compute the values list, and they might not follow the sort order from the root nodes.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
79.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[triceo]

Without running any benchmarks, I can already anticipate several places where performance will suffer compared to the previous solution. IMO all of them are fixable, and may also lead to significantly simpler code.