Adding a tutorial for graph regularization with images

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

Adding a tutorial for graph regularization with images #98

Closed sayakpaul closed 2 years ago

sayakpaul commented 3 years ago

@arjung I reckoned that adding the example under g3docs/tutorials might be more appropriate because it would be the first one that shows how to build a synthetic graph for an image dataset and build a model.

Let me know your thoughts. A parallel Colab Notebook is available here for the results. For reference, the corresponding issue thread is available here.

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

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

sayakpaul commented 3 years ago

@arjung thanks for your feedback but I am not comfortable with moving this to the location you suggested. The primary reason is visibility.

So, if the team decides to consider that later at some point in time we can keep this PR open otherwise, closing the PR seems to be the only option.

arjung commented 3 years ago

I thought this was something we agreed upon prior to starting work on this tutorial (see https://github.com/tensorflow/neural-structured-learning/issues/87#issuecomment-871964966).

Here are a few of our reasons for keeping it in examples/notebooks for now:

we want to let the tutorial mature for some time
tutorials shown on the TF website directly impose additional maintenance work for our team because these tutorials go through regular testing and underlying TF changes can often break them
it's harder to remove a tutorial from our website than it is to add a new one

As mentioned in https://github.com/tensorflow/neural-structured-learning/issues/99, I am planning to add a link to the examples directory from our TF website. So, that should address some of the visibility concerns.

sayakpaul commented 3 years ago

we want to let the tutorial mature for some time

Can you expand a bit more on this?

If the notebook is going to be hosted on that directory, I'd prefer to add my name (as the author) just after the notebook title. Happy to add acknowledgments too. Here are a few other suggestions:

A short README here to clarify the purpose/objectives of the directory so that the readers understand what differentiates the content hosted there from the tutorials. Would also make sense to add one-liners for all the major notebooks/scripts to specify what they are all about. This makes the navigation experience better.
Mentioning the examples directory from the README preferably after this section:

I think this is particularly important. Otherwise, there's no way a user could know about it from the repository.
Making it clear from this section on Contribution. I understand if different tutorial proposals would have different scopes but it still helps to be aware of all these before a proposal.

arjung commented 3 years ago

Yeah, feel free to list your name and add other contributors/acknowledgments to the tutorial. That shouldn't be an issue.

And thanks for the other suggestions! Will address them too.

sayakpaul commented 3 years ago

Thanks for being so welcoming and understanding. I will proceed now and address your feedback. But I still need to know this bit:

we want to let the tutorial mature for some time

Can you expand a bit more on this?

arjung commented 3 years ago

Of course! Thanks for your contributions! Like I mentioned earlier, it's easier to add tutorials to the website than it is to remove them once they are added. Keeping new additions in a separate directory gives us time to understand how much churn there is, if any, to the tutorial, get feedback from other users potentially, improve upon it, and so on. All this and the maintenance work for our team. Keeping this in the examples directory doesn't preclude the possibility of moving it or other tutorials to our TF website in the future. We'll definitely reach out to you if we decide to do that in the future.

sayakpaul commented 3 years ago

Understood. Thanks, Arjun!

sayakpaul commented 3 years ago

@arjung I have addressed all your comments. Over to you.

arjung commented 2 years ago

Hi Sayak, I was on vacation for the past week and just got back today. Will look at the latest diff within the next couple of days.

sayakpaul commented 2 years ago

@arjung not sure why the notebook commenting was troublesome but I appreciate you being thorough with the comments.

I have addressed all the comments except for this one:

Do you use unlabeled examples or not in the example? The invocation to pack_nbrs() suggests that you don't but in the text before that, you mention that you do. So it's a bit confusing.

I don't think I mentioned this. However, if you could point me to any specific line that indicates this, happy to look into that again.

arjung commented 2 years ago

Do you use unlabeled examples or not in the example? The invocation to pack_nbrs() suggests that you don't but in the text before that, you mention that you do. So it's a bit confusing.

I don't think I mentioned this. However, if you could point me to any specific line that indicates this, happy to look into that again.

Sorry my bad. I misread 'undirected' as 'unlabeled' in the text before the call to pack_nbrs(). Ignore this comment. Everything looks good now!