Skip to content

Conversation

copybara-service[bot]
Copy link

Fix TFMOT failure

FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774

FUTURE_COPYBARA_INTEGRATE_REVIEW=#508 from benkli01:toupstream/clustering-visualization 5d75774
PiperOrigin-RevId: 330602592
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

@googlebot googlebot added the cla: yes PR contributor has signed CLA label Sep 8, 2020
@github-actions github-actions bot added technique:clustering Regarding tfmot.clustering.keras APIs and docs technique:pruning Regarding tfmot.sparsity.keras APIs and docs technique:qat Regarding tfmot.quantization.keras (for quantization-aware training) APIs and docs labels Sep 8, 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.

From the description it appears that this PR is to fix failures in #508. #508 is now based on the top of #516, merged earlier. However, this PR does not include changes from #516. Can you please rebase it?

@benkli01, could you please have a look as well, and check if the above makes sense.

@benkli01
Copy link
Contributor

benkli01 commented Sep 9, 2020

@akarmi You are right. The changes from #516 seem to be missing in this PR. I will have a look.

Copy link
Contributor

@benkli01 benkli01 left a comment

Choose a reason for hiding this comment

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

Hi @alanchiao,
as mentioned by Anton above this PR seems to contain changes of #508 before it was rebased onto #516 yesterday, i.e. it seems to be outdated. Also there are some changes to jupyter notebooks (unrelated to #508) included in this PR that seem to revert the changes from #512 among other things.

Could you please have a look and either just rebase this PR onto master or resolve it in some other fashion?

@alanchiao
Copy link

Thanks Anton and Benjamin. I'm taking a look into what happened in the PR merging system that caused this issue to happen and try have #508 properly merged.

@alanchiao
Copy link

@yashk2810 (author of this PR) for visibility on my comment

@Ruomei
Copy link
Contributor

Ruomei commented Sep 9, 2020

Hi @akarmi, is it urgent? sorry that I might not be able to review it very soon. I’ve been feeling unwell since the beginning of my holiday. However, will def take a look when I feel better.

@akarmi
Copy link
Contributor

akarmi commented Sep 10, 2020

Hi @akarmi, is it urgent? sorry that I might not be able to review it very soon. I’ve been feeling unwell since the beginning of my holiday. However, will def take a look when I feel better.

Don't worry about this Ruomei. I believe Alan is looking at the underlying issue on his side. I don't think anything is needed from us atm.

Get better soon!

@copybara-service copybara-service bot closed this Sep 10, 2020
@copybara-service copybara-service bot deleted the test_330602592 branch September 10, 2020 22:37
@Ruomei
Copy link
Contributor

Ruomei commented Sep 11, 2020

Hi @akarmi, is it urgent? sorry that I might not be able to review it very soon. I’ve been feeling unwell since the beginning of my holiday. However, will def take a look when I feel better.

Don't worry about this Ruomei. I believe Alan is looking at the underlying issue on his side. I don't think anything is needed from us atm.

Get better soon!

Thanks!

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 technique:clustering Regarding tfmot.clustering.keras APIs and docs technique:pruning Regarding tfmot.sparsity.keras APIs and docs technique:qat Regarding tfmot.quantization.keras (for quantization-aware training) APIs and docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants