hwu2024 commented on PR #3187: URL: https://github.com/apache/incubator-kie-optaplanner/pull/3187#issuecomment-3614775829
Hi @Rikkola thanks for the review! The core problem is that `setupValidNonsequential4OptMove` used `Collectors.toMap()` with the default `HashMap`, and then iterated `entityToListSize.entrySet()`. That makes the per‑entity lists depend on the `HashMap` iteration order, which `HashMap` [makes no guarantees as to the order of the map](https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html). NonDex exposes this by shuffling that iterator and we can also trace the error by running the debug mode: ``` mvn -pl core/optaplanner-core-impl edu.illinois:nondex-maven-plugin:2.2.1:debug -Dtest=org.optaplanner.core.impl.heuristic.selector.move.generic.list.kopt.KOptListMoveIteratorTest#testNonsequentialKOptOnDifferentEntity ``` which the debug stack shows that ``` java.util.HashMap$EntrySet.iterator(HashMap.java:1106) org.optaplanner.core.impl.heuristic.selector.move.generic.list.kopt.KOptListMoveIteratorTest.setupValidNonsequential4OptMove(KOptListMoveIteratorTest.java:233) ``` We can also reproduce by manually building two maps with the same keys/values but different iteration order (simulating `HashMap`), and runs the same list‑building logic as `setupValidNonsequential4OptMove`. ``` @Test void entityToListDependsOnMapIterationOrder() { int k = 4; Object[] data = new Object[2 * k + 8]; for (int i = 0; i < data.length; i++) { data[i] = "v" + i; } Map<Object, Integer> entityToListSizeOrder1 = new LinkedHashMap<>(); entityToListSizeOrder1.put("e1", 6); entityToListSizeOrder1.put("e2", 4); Map<Object, Integer> entityToListSizeOrder2 = new LinkedHashMap<>(); entityToListSizeOrder2.put("e2", 4); entityToListSizeOrder2.put("e1", 6); Map<Object, List<Object>> entityToList1 = buildEntityToListLikeNonsequentialHelper(entityToListSizeOrder1, data); Map<Object, List<Object>> entityToList2 = buildEntityToListLikeNonsequentialHelper(entityToListSizeOrder2, data); assertThat(entityToList1).isEqualTo(entityToList2); } private static Map<Object, List<Object>> buildEntityToListLikeNonsequentialHelper( Map<Object, Integer> entityToListSize, Object[] data) { Map<Object, List<Object>> entityToList = new HashMap<>(); int offset = 0; for (Map.Entry<Object, Integer> entry : entityToListSize.entrySet()) { Object entity = entry.getKey(); int listSize = entry.getValue(); List<Object> entityList = new ArrayList<>(Arrays.asList(data).subList(offset, offset + listSize)); for (int i = 0; i < listSize; i++) { entityList.add(2 * i + 1, entity + "-extra-" + i); } entityList.add(0, entity + "-start"); entityList.add(entity + "-end"); entityToList.put(entity, entityList); offset += listSize; } return entityToList; } ``` and if we run ``` mvn -pl core/optaplanner-core-impl test -Dtest=org.optaplanner.core.impl.heuristic.selector.move.generic.list.kopt.KOptListMoveIteratorTest#entityToListDependsOnMapIterationOrder ``` we get ``` expected: {"e1"=["e1-start", "v4", "e1-extra-0", "v5", "e1-extra-1", "v6", "e1-extra-2", "v7", "e1-extra-3", "v8", "e1-extra-4", "v9", "e1-extra-5", "e1-end"], "e2"=["e2-start", "v0", "e2-extra-0", "v1", "e2-extra-1", "v2", "e2-extra-2", "v3", "e2-extra-3", "e2-end"]} but was: {"e1"=["e1-start", "v0", "e1-extra-0", "v1", "e1-extra-1", "v2", "e1-extra-2", "v3", "e1-extra-3", "v4", "e1-extra-4", "v5", "e1-extra-5", "e1-end"], "e2"=["e2-start", "v6", "e2-extra-0", "v7", "e2-extra-1", "v8", "e2-extra-2", "v9", "e2-extra-3", "e2-end"]} ``` It shows that the old logic using default `HashMap` was order‑sensitive. The `LinkedHashMap` change in this PR then removes that bug. Thanks again for your time to follow up! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
