Skip to content

Conversation

benkli01
Copy link
Contributor

@benkli01 benkli01 commented Aug 6, 2020

Note: Batch-wise summaries of the clustering can be slow and might lead to a warning like this:

"WARNING:tensorflow:Method (on_train_batch_end) is slow compared
 to the batch update (0.102097). Check your callbacks."

@googlebot googlebot added the cla: yes PR contributor has signed CLA label Aug 6, 2020
@alanchiao alanchiao added the technique:clustering Regarding tfmot.clustering.keras APIs and docs label Aug 14, 2020
Copy link
Contributor

@akarmi akarmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Just a couple of minor things.

@benkli01 benkli01 force-pushed the toupstream/clustering-visualization branch 2 times, most recently from b105810 to 5d75774 Compare August 24, 2020 09:38
Copy link
Contributor

@akarmi akarmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@akarmi akarmi added the ready to pull Working to get PR submitted to internal repository, after which merging to Github happens. label Aug 25, 2020
@akarmi
Copy link
Contributor

akarmi commented Sep 8, 2020

@benkli01, please resolve merge conflict

Note:
Batch-wise summaries of the clustering can be slow and might lead to a
warning like this:
    "WARNING:tensorflow:Method (on_train_batch_end) is slow compared
     to the batch update (0.102097). Check your callbacks."
@benkli01 benkli01 force-pushed the toupstream/clustering-visualization branch from 5d75774 to 6e4ae44 Compare September 8, 2020 09:37
@benkli01
Copy link
Contributor Author

benkli01 commented Sep 8, 2020

Merge conflict resolved and commit message adapted, because the simplification of the example (#516) is already merged.

copybara-service bot pushed a commit that referenced this pull request Sep 8, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 330602592
@copybara-service copybara-service bot mentioned this pull request Sep 8, 2020
copybara-service bot pushed a commit that referenced this pull request Sep 10, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331028556
copybara-service bot pushed a commit that referenced this pull request Sep 10, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331026941
copybara-service bot pushed a commit that referenced this pull request Sep 10, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331028556
copybara-service bot pushed a commit that referenced this pull request Sep 11, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331031582
copybara-service bot pushed a commit that referenced this pull request Sep 11, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331031582
copybara-service bot pushed a commit that referenced this pull request Sep 11, 2020
FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 331031582
alanchiao pushed a commit that referenced this pull request Sep 11, 2020
--
5d75774 by Benjamin Klimczak <benjamin.klimczak@arm.com>:

Add visualization output via tensorboard to the clustering example.

Note:
- The current example seems to be overparameterized, so that there is not
  very much to see in the visualization. This should be addressed by another
  PR making the example leaner.
- Writing the visualization summaries batch-wise leads to a warning
  message like this:
    "WARNING:tensorflow:Method (on_train_batch_end) is slow compared
     to the batch update (0.102097). Check your callbacks."
  It should be further investigated if this can be avoided.
COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 330547925
@alanchiao
Copy link

The commit has been added to Github (see here) and attributed to Benjamin. Unfortunately, due to the hiccup, I don't think we can ever mark the PR itself as merged.

The cause was that I went through the regular merge procedure, but there was an issue that showed up after merging, from an internal check that makes sure that there is the "COPYRIGHT" part at the top of every file. I'm changing things so that the check will run earlier during "import/copybara" and this issue won't arise again. Apologize for the trouble.

@alanchiao
Copy link

Closing this given the above comment. The merged commit is here: 5603678.

@alanchiao alanchiao closed this Sep 11, 2020
@benkli01
Copy link
Contributor Author

Thanks for taking care of this, @alanchiao !

@benkli01
Copy link
Contributor Author

Some of the latest changes of this PR seem to be missing in the commit to the master branch. I will have a closer look and, if needed, I will just create a new PR to amend this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes PR contributor has signed CLA ready to pull Working to get PR submitted to internal repository, after which merging to Github happens. technique:clustering Regarding tfmot.clustering.keras APIs and docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants