Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add xfail test for SavedModel restoring optimizer weights #15661

Closed

Conversation

adriangb
Copy link
Contributor

No description provided.

@adriangb
Copy link
Contributor Author

@bhack could you run kokoro please?

@bhack
Copy link
Contributor

bhack commented Nov 18, 2021

Sorry, I cannot assign labels in this repository

@gbaned gbaned requested a review from qlzh727 November 18, 2021 04:01
@chenmoneygithub chenmoneygithub requested review from k-w-w and removed request for qlzh727 November 18, 2021 18:32
@gbaned gbaned added stat:awaiting keras-eng Awaiting response from Keras engineer keras-team-review-pending Pending review by a Keras team member. and removed stat:awaiting keras-eng Awaiting response from Keras engineer labels Nov 19, 2021
@mattdangerw mattdangerw removed the keras-team-review-pending Pending review by a Keras team member. label Dec 2, 2021
@adriangb
Copy link
Contributor Author

Anything I can do here to get the ball rolling?

@gbaned gbaned requested a review from fchollet December 31, 2021 18:09
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Dec 31, 2021
@qlzh727
Copy link
Member

qlzh727 commented Jan 4, 2022

@k-w-w, can you take a look for this PR? Thanks.

@qlzh727 qlzh727 removed the keras-team-review-pending Pending review by a Keras team member. label Jan 4, 2022
@adriangb
Copy link
Contributor Author

Hi folks, just checking in so this doesn't go stale. It'd be great to make some progress on this long-standing footgun

" optimizer weights"
)
else:
self.assertAllClose(model.optimizer.weights,
Copy link
Contributor

@bhack bhack Jan 25, 2022

Choose a reason for hiding this comment

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

Honestly I would move this in a separate test function decorating it like in https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/eager/def_function_test.py#L243 with a reference to the Github ticket.

So when it is implemented the test will fail and we have a pointer to the original issue/feature request.
Then we could remove the decorator or simplify/unify the original test (the current method).

Copy link
Contributor

Choose a reason for hiding this comment

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

@adriangb Can you handle this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I'll have bandwidth anytime soon

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem the comment itself is 6 months old

@gbaned
Copy link
Collaborator

gbaned commented Jun 10, 2022

@adriangb Can you please resolve conflicts? Thank you!

@adriangb
Copy link
Contributor Author

I'd like someone to approve this / give me some indication that this won't just sit here for another 8 months before I spend the time to resolve merge conflicts.

@gbaned
Copy link
Collaborator

gbaned commented Aug 5, 2022

I'd like someone to approve this / give me some indication that this won't just sit here for another 8 months before I spend the time to resolve merge conflicts.

@adriangb Sorry for the delay, once conflicts resolved, we will review it quickly. Thank you!

@gbaned
Copy link
Collaborator

gbaned commented Dec 16, 2022

Hi @adriangb Any update on this PR? Please. Thank you!

@gbaned
Copy link
Collaborator

gbaned commented Mar 21, 2023

Hi @adriangb I'm going to go ahead and close this PR, because it seems to have stalled. If you're still interested in pursing this (and responding to my comments), please feel free to reopen! Thank you!

@gbaned gbaned closed this Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants