search

dorimedini / robustness_properties

GLOBIGLOBIGLOBI
0 stars 0 forks source link

Normalize threshold when pruning #93

Closed dorimedini closed 5 years ago

dorimedini commented 5 years ago

Solves #91

This PR also improves debug logging of pruning process (outputs % of edges pruned) so I also added a default value for threshold in WinneryExperiment (0.001).

Can either of you make sense of the results...? Percent of edges pruned is 1 minus winnery intersection ratio (which is what it should be, right?)

image

galshachaf commented 5 years ago

for better readability, I think you should use a scatter plot

galshachaf commented 5 years ago

what about l1 and l-inf norms?

dorimedini commented 5 years ago

what about l1 and l-inf norms?

Done, added commit to #92

dorimedini commented 5 years ago

for better readability, I think you should use a scatter plot

Done, added commit to #92

dorimedini commented 5 years ago

image

@galshachaf

galshachaf commented 5 years ago

winning ticket intersection is the almost 1.0 for all values? That's not really something we can work with :)

On Thu, 15 Aug 2019 at 00:21, dorimedini notifications@github.com wrote:

[image: image] https://user-images.githubusercontent.com/3027929/63057270-88eb5d00-bef2-11e9-83de-2b1d4292cc40.png

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/dorimedini/kardashigans/pull/93?email_source=notifications&email_token=ADHINCF6DKKOTW5XECPPVK3QERZL5A5CNFSM4ILOT7OKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4KEUMQ#issuecomment-521423410, or mute the thread https://github.com/notifications/unsubscribe-auth/ADHINCARAYFKGYUJJYHRYN3QERZL5ANCNFSM4ILOT7OA .

dorimedini commented 5 years ago

winning ticket intersection is the almost 1.0 for all values? That's not really something we can work with :) … On Thu, 15 Aug 2019 at 00:21, dorimedini @.***> wrote: [image: image] https://user-images.githubusercontent.com/3027929/63057270-88eb5d00-bef2-11e9-83de-2b1d4292cc40.png — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#93?email_source=notifications&email_token=ADHINCF6DKKOTW5XECPPVK3QERZL5A5CNFSM4ILOT7OKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4KEUMQ#issuecomment-521423410>, or mute the thread https://github.com/notifications/unsubscribe-auth/ADHINCARAYFKGYUJJYHRYN3QERZL5ANCNFSM4ILOT7OA .

Looks buggy, with 0.001 threshold we prune at least 25% of edges in both datasets. Did you see any issues during review? Maybe I'm computing the winnery ticket wrong?

galshachaf commented 5 years ago

@dorimedini did you solve this issue?

dorimedini commented 5 years ago

@dorimedini did you solve this issue?

Nope. I didn't burn hours on it though, on Saturday maybe I'll have some time @galshachaf

dorimedini commented 5 years ago

@dorimedini did you solve this issue?

Nope. I didn't burn hours on it though, on Saturday maybe I'll have some time @galshachaf

@galshachaf after rebasing this branch on the winnery-experiment-should-output-l2-diffs branch (#92) the output looks much better. Seems like a non-existent bug that will go away when both PRs are merged. Here are the most recent results:

image

dorimedini commented 5 years ago

@galshachaf @noamloya can I merge this?