Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAMOA-73: fixes NullPointerException in VerticalHoeffdingTree #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgrzenda
Copy link
Contributor

java.lang.NullPointerException is in some cases thrown in org.apache.samoa.learners.classifiers.trees.ModelAggregatorProcessor.process(ModelAggregatorProcessor.java:142)
The problem occurs because FoundNode objects in some cases contain null node property. Such FoundNode objects can be created and are considered valid, which directly follows from filterInstanceToLeaf(Instance inst, SplitNode parent, int parentBranch) method of SplitNode class.
However, once a categorical feature is selected for the splits, the following situation can happen:

  1. first FoundNode object with null node object inside is created
  2. Next, the tree structure is grown by establishing new LearningNode, which is done in ModelAggregatorProcessor.java/trainOnInstanceImpl(FoundNode foundNode, Instance inst)
  3. The problem is the LearningNode is not added to FoundNode helper object already existing at this stage, which causes NullPointerException exception when trying to use it.

Solution: In such cases, the FoundNode object should reflect the newly created tree extension. This can be done by injecting into the FoundNode the newly created Node object. This is to keep FoundNode in sync with the most recent tree structure. This is what this fix makes: it replaces null node object with the newly created one.

Copy link
Contributor

@csterling csterling left a comment

Choose a reason for hiding this comment

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

+1, well spotted.

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.

2 participants