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

show_just_path_prediction #70

Closed
tlapusan opened this issue Nov 28, 2019 · 15 comments · Fixed by #71
Closed

show_just_path_prediction #70

tlapusan opened this issue Nov 28, 2019 · 15 comments · Fixed by #71
Milestone

Comments

@tlapusan
Copy link
Collaborator

I started to work on showing just the prediction path (based on what we discussed in a previous issue)

Right now, it looks like this :
Screenshot 2019-11-28 at 15 01 21

Am I on the right track ? :) Do you think we need to show also the neighborhood nodes. I don't have a strong argument for them, but I like how they look (they show somehow the 'opposite' prediction path)

@parrt
Copy link
Owner

parrt commented Nov 28, 2019

Looks pretty cool! I think our goal, though, is to show the path from root to predictor leaf in a big tree as tightly as possible. I wonder what it looks like as a vertical stack of just the decision nodes entered and then the leaf. That would be the smallest footprint, which I think is kind of the goal here. what do you think?

@tlapusan
Copy link
Collaborator Author

Indeed, if I look again at above visualisation, it looks 'scattered'. I will adjust it to the vertical stack version. Let's see how it'll look ;)

@tlapusan
Copy link
Collaborator Author

It looks much cleaner now :)
Screenshot 2019-11-28 at 22 55 03

@parrt
Copy link
Owner

parrt commented Nov 28, 2019

heh, nice work! I like it. Does it work horizontally too? I think we have an option for that.

@tlapusan
Copy link
Collaborator Author

cool. By horizontally.... do you mean a horizontal stack instead of vertical stack ?

@parrt
Copy link
Owner

parrt commented Nov 28, 2019

yep, left to right

@tlapusan
Copy link
Collaborator Author

wouldn't be a problem if the prediction path is very deep ? I think we will end with a 'crowdy' plot.

@parrt
Copy link
Owner

parrt commented Nov 28, 2019

well, sometimes you want very tall and sometimes very wide. seems like original code can change orientation so this should inherit that; i assume you just cut/paste, right?

@tlapusan
Copy link
Collaborator Author

I wanted to use as much functionality as possible from dtreeviz(), so I didn't cut/paste. I will take a look for horizontal view. There is little work left also for vertical view, I made a little 'hack' to create the visualisation with vertical view for decision nodes only :)

@parrt
Copy link
Owner

parrt commented Nov 29, 2019

We can skip horizontal if it gets messy in the code :)

@tlapusan
Copy link
Collaborator Author

tlapusan commented Nov 30, 2019

For horizontal view I didn't have to do any extra logic, it was already in dtreeviz() code ;)

Should I make a PR ?

Screenshot 2019-11-30 at 19 02 33

Screenshot 2019-11-30 at 19 03 22

@parrt
Copy link
Owner

parrt commented Nov 30, 2019

Looks great! Only question is, how do we activate this? Seems like it should be an arg to existing tree viz functions. Oh, I see you have show_just_path. Sounds good! Please do make a PR. Does it work for regression trees too?

@tlapusan
Copy link
Collaborator Author

I didn't try it on regression trees. I will try it tomorrow.

@tlapusan
Copy link
Collaborator Author

tlapusan commented Dec 1, 2019

yey, it works also for regression trees. I will create a PR for this.

Screenshot 2019-12-01 at 16 59 27

@parrt
Copy link
Owner

parrt commented Dec 1, 2019

great job!

@parrt parrt closed this as completed in #71 Dec 1, 2019
@parrt parrt added this to the 0.8 milestone Dec 4, 2019
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