tensorflow / neural-structured-learning

Training neural models with structured signals.
https://www.tensorflow.org/neural_structured_learning
Apache License 2.0
980 stars 189 forks source link

Added CNN adversarial learning tutorial notebook #67

Closed dipanjanS closed 3 years ago

dipanjanS commented 4 years ago

Hi @arjung as discussed. Please find the adversarial learning notebook example.

I have added it in the neural_structured_learning/examples/notebooks as you had mentioned. Feel free to suggest the necessary edits and changes to keep in line with Google & TF standards and I will make the necessary changes based on feedback.

review-notebook-app[bot] commented 4 years ago

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

dipanjanS commented 4 years ago

@googlebot I signed it!

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

dipanjanS commented 3 years ago

Thanks, let me get to this by this week and update with a corrected version

arjung commented 3 years ago

Hi @dipanjanS, just wanted to check in and see if you had time to work on this PR following my comments.

dipanjanS commented 3 years ago

@arjung yep have worked on the changes you proposed, let me push an updated version soon and will tag you

google-cla[bot] commented 3 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot commented 3 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

dipanjanS commented 3 years ago

@arjung for some reason a lot of older files seem to have come up in this commit from other folders even though I rebased. Not sure why this came up, let me create a fresh PR and tag it here

dipanjanS commented 3 years ago

Hi @arjung please find the updated notebook in this fresh PR: https://github.com/tensorflow/neural-structured-learning/pull/76 and you can close this one. Apologies on creating a fresh one, for some reason rebase seemed to show up some older files as ones to be committed which I didn't modify.

arjung commented 3 years ago

Thanks for working on it! I think (but am not 100% sure) that rebasing in general after a review has started will make it impossible to do incremental reviews. A new PR also means that we'll have lost the history of comments/replies from this PR, and again no way to do an incremental review :(.

Btw some of comments still don't have a response. Could you take a look at them and let me know if/how you acted on them? That'll help me review the new PR.

dipanjanS commented 3 years ago

@arjung sure thanks for taking the time and apologies not sure why this happened :( I will double check next time before pushing the commit if necessary

I have commented on all your review comments above and made the necessary changes in the other PR.

arjung commented 3 years ago

Thanks, will get to the new PR before the end of the week.

csferng commented 3 years ago

Closing this PR since new updates are in #76.