Bug 1118784 - NPE when getBestScoreImproved method is called from AbstractStepScope during ConstructionHeuristic
Summary: NPE when getBestScoreImproved method is called from AbstractStepScope during ...
Alias: None
Product: JBoss BRMS Platform 6
Classification: Retired
Component: OptaPlanner
Version: 6.0.2
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
: ---
Assignee: Geoffrey De Smet
QA Contact: jvahala
Depends On:
TreeView+ depends on / blocked
Reported: 2014-07-11 14:10 UTC by jvahala
Modified: 2016-04-28 05:08 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2014-07-14 12:27:44 UTC
Type: Bug

Attachments (Terms of Use)

Description jvahala 2014-07-11 14:10:29 UTC
Description of problem:
When I register my own step listener and I ask AbstractStepScope if score was improved NPE is thrown during Construction Heuristic phase. The problem is that  bestScoreImproved attribute is initialized after first LocalSearch step. But technically it should be false from the beginning because ConstructionHeuristic always lowers score. 

How reproducible:
Extends SolverPhaseLifecycleListener and in stepEnded method use for example really simple IF:  if(stepScope.getBestScoreImproved().booleanValue())

Actual results:
NPE is thrown

Expected results:
result of calling .getBestScoreImproved().booleanValue() should be false

Comment 2 Geoffrey De Smet 2014-07-11 14:23:23 UTC
This is a known issue (which might not be solvable even). Note that LocalSearchPhaseLifecycleListener (unlike SolverEventListener) isn't part of the public API because they are deep inside the optaplanner-core. Despite that, the issue is valid of course.

In every step, the Construction Heuristics initialize 1 entity. Because this causes the uninitializedEntityCount to decrease, this actually means that the best score improved(!). Think about it, a solution with score -7 and 10 unassigned nurses is better than a solution with score 0 and 20 unassigned nurses.
However, this would imply that best solution changed, so the working solution needs to be cloned and an best solution event needs to be fired. That cloning is a huge performance killer. So CH's don't do that, they delay that until the last step...
Meanwhile, the bestSolutionImproved and other related field stay null to fail fast if any code is using them (which is better than holding the wrong non-null value...)

Comment 3 Geoffrey De Smet 2014-07-14 12:19:49 UTC
Going to reject this issue because reasons stated above and more importantly because the code does:
  ((DefaultSolver) solver).addSolverPhaseLifecycleListener(listener);

Downcasting to DefaultSolver, to be able to mess with the internal classes is not supported.

Meanwhile, Jiri's PR will improve the javadoc for those that do mess with the internal classes. Also, we 're investigating why there was the need to mess with the internal classes in the first place, and if we remove that need by supporting a new requirement on the public api.

Note You need to log in before you can comment on or make changes to this bug.