Skip to content

activations, constraints, initializers, losses, regularizers: move Ops param from CTOR to call method #329

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

Merged
merged 25 commits into from
Jun 2, 2021

Conversation

JimClarke5
Copy link
Contributor

This PR moves the Ops parameter out of the class CTORs into their respective call methods. Also, an interface was created for each package for the respective call method. These packages were chosen because they are stateless wrt the Graph (e.g. no Variables).

There is no change in functionality of these classes, just the move of the Ops parameter to call.

This is related to issue: #327

karllessard
karllessard previously approved these changes Jun 1, 2021
@karllessard
Copy link
Collaborator

Will merge after #331, if the quick-build passes.

@karllessard
Copy link
Collaborator

@JimClarke5 , can you rebase your PR so that we can validate now that the quick-build is passing?

@JimClarke5
Copy link
Contributor Author

@karllessard I think I have rebased it. Give it a try.

@karllessard
Copy link
Collaborator

karllessard commented Jun 2, 2021

It failed again, looks like it’s the javadoc trouble that is back :-|

If I remember correctly, the fix for that was to enforce building in Java8, that has not changed as far as I know

Wait, I think I'm confusing things... I'll try to demystify that later

@karllessard
Copy link
Collaborator

I see that you merged master to your branch, I don’t think that’s the issue but rebasing is always better. You can drop your last commit (the merge one) and retry with git rebase?

@karllessard
Copy link
Collaborator

Oh wait, in fact, these javadoc errors are real errors this time :)

Can you please fix them and push a new version?

@JimClarke5
Copy link
Contributor Author

@karllessard @rnett The "check format" is erroring on lines between imports. I ran google-java-format against all my files so I don't understand why this is happening.

@karllessard
Copy link
Collaborator

The way to reformat now is to run mvn spotless:apply, can you give it a try?

@JimClarke5
Copy link
Contributor Author

@karllessard I did the mvn spotless:apply, looks check-format passed this time.

@karllessard karllessard merged commit 23d6f0b into tensorflow:master Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants