-
Notifications
You must be signed in to change notification settings - Fork 150
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
Fix relative cav #111
base: master
Are you sure you want to change the base?
Fix relative cav #111
Conversation
- the paper states in section 3.6 for relative tcav, construct a positive and negative set, e.g., for concepts P_dot, P_stripe, P_mesh this would look like, P_stripe (positive) and {P_dot U p_mesh} (negative). However, the code was giving sets like, P_stripe (pos), P_dot (neg). Now, the complement set is used for relative tcav as described in the paper. - when computing relative tcav, plot function failed because concepts are also 'random' concepts and is_random_concept(concept) is always true, therefore. Now, plot for relative tcav handles 'random' concepts correctly.
Thank you for submitting this RP @monz ! Two things if you are able 1) mind changing the relative_cav variable to be is_relative_cav (to avoid confusion that the variable IS the cav) 2) this is big enough change that I feel a few tests could be very helpful. Would you mind adding a few simple ones to our test deck? Thank you! |
Hi @BeenKim, please have a look to the added tests, hope this is what you expected, kind regards, Markus. |
Hi @BeenKim I know its kind of a big PR, however, I am curious what you think about it. Kind Regards, Markus. |
Hi Markus!
Thank you for incorporating our request! I'm excited to review this, but
having trouble finding time to test the new changes end-to-end.
Hopeful we'll get to this soon.
Cheers
Been
…On Fri, Jun 11, 2021 at 12:19 AM Markus Monz ***@***.***> wrote:
Hi @BeenKim <https://github.com/BeenKim> I know its kind of a big PR,
however, I am curious what you think about it. Kind Regards, Markus.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#111 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAP35E35ABRBEIG4H6FP5U3TSG2GHANCNFSM44NZHAOA>
.
|
Hope I did understand the paper correctly and gave a proper fix.