-
Notifications
You must be signed in to change notification settings - Fork 215
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
Default loss function should use log ratio, not subtraction #137
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We're using your work for an upcoming release of cbioportal.org. We wanted more control over the rendering, so we used your exposed algorithms for computing layout, and manually rendering the circles using that layout. This has been a godsend, thank you.
But I discovered an issue - sometimes, in a particularly tight layout situation, the circles would layout so as to appear that an actually empty intersection is not empty.
![image](https://user-images.githubusercontent.com/636232/57811887-6a31ca80-7739-11e9-9a33-1a9b32651155.png)
For example:
I was able to fix this on our end by copying over the loss function but modifying this line
https://github.com/benfred/venn.js/blob/master/src/layout.js#L416
to turn it into
![image](https://user-images.githubusercontent.com/636232/57811685-f42d6380-7738-11e9-8fd5-645599e24c7a.png)
I believe this is better because as described in the screenshot, it weights small sets equally. I don't have time to open a PR but I wanted to drop this idea in because I think it's a good one.
The result is:
![image](https://user-images.githubusercontent.com/636232/57811903-73bb3280-7739-11e9-9b21-2fd8f4e4d15e.png)
The text was updated successfully, but these errors were encountered: