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

Odd Behavior for nrAssignments in DecisionTree #1496

Closed
varunagrawal opened this issue Mar 22, 2023 · 0 comments · Fixed by #1501
Closed

Odd Behavior for nrAssignments in DecisionTree #1496

varunagrawal opened this issue Mar 22, 2023 · 0 comments · Fixed by #1501
Assignees

Comments

@varunagrawal
Copy link
Collaborator

When creating a decision tree from a vector of double values, depending on the ordering of the keys, the number of assignments can be incorrect.

For example, this

DiscreteKeys err_d0{{M(1), 2}, {M(0), 2}};
std::vector<double> p1 = {0, 0, 1, 2};
AlgebraicDecisionTree<Key> error_dt(err_d0, p1);
error_dt.print("", DefaultKeyFormatter);

gives

Choice(m1)  [Different]
 0 Leaf    0, (2)
 1 Choice(m0)  [Different]
 1 0 Leaf    1, (1)
 1 1 Leaf    2, (1)

which looks good.

Note that I have added the nrAssignments for each leaf in parenthesis, so 0 Leaf 0, (2) specifies that there is 1 leaf for m1=0, m0=0 and it has 2 assignments.

However, when I specify the following tree (where only the discrete keys are reversed in order),

DiscreteKeys err_d0{{M(0), 2}, {M(1), 2}};
std::vector<double> p1 = {0, 0, 1, 2};
AlgebraicDecisionTree<Key> error_dt(err_d0, p1);
error_dt.print("", DefaultKeyFormatter);

I get

Choice(m1)  [Different]
 0 Choice(m0)  [Different]
 0 0 Leaf    0, (2)
 0 1 Leaf    1, (1)
 1 Choice(m0)  [Different]
 1 0 Leaf    0, (2)
 1 1 Leaf    2, (1)

where the number of assignments are incorrect. It should be 1 each so that counting gives rise to a total of 4.

The issue seems to be that DecisionTree::compose reorders the keys such that the highest key value is always at the top, but the number of assignments are not updated accordingly.

The question at hand is whether we require the key reordering when composing the tree? What would be the rationale behind this?

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 a pull request may close this issue.

2 participants