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

Regularization: adding weight decay to Loss #788

Merged
merged 4 commits into from
Mar 29, 2021

Conversation

hmf
Copy link
Contributor

@hmf hmf commented Mar 25, 2021

Description

As per the discussion here I have:

  • Added class for weigh decay: L1WeightDecay, L2WeightDecay and ElasticNetWeightDecay
  • Added a test class WeightDecayTest with 3 tests (one for each class above)

All tests are in the loss package. Don't think this interferes with anything else in the project. I also did not find additional tests using SimpleCompositeLoss so I did not add these.

@hmf
Copy link
Contributor Author

hmf commented Mar 25, 2021

Continuous Linux build fail is unrelated to changes. Anyway we can run this again?

@aksrajvanshi
Copy link
Contributor

aksrajvanshi commented Mar 25, 2021

Continuous Linux build fail is unrelated to changes. Anyway we can run this again?

Hi @hmf , you need to generate a proper javadoc comments for the functions you created.

For e.g. for public ElasticNetWeightDecay(String name, NDList parameters, float lambda), you need to define what the parameter lambda is for in the comments above.

The build system checks if you defined all the parameters properly or not. Ensure you write those and run ./gradlew :api:javadoc, ./gradlew fJ, ./gradlew compileJava and look out for any errors before pushing the changes.

@hmf
Copy link
Contributor Author

hmf commented Mar 25, 2021

Seems to be ok now (see at the end).

Don't now if it is intended, but I got warnings and not errors. Also, couldn't we add this to the tests and generate errors?

TIA

:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

> Task :api:javadoc
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/ElasticNetWeightDecay.java:67: warning - Parameter "lambda1" is documented more than once.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L1WeightDecay.java:49: warning - @param argument "weight" is not a parameter name.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L2WeightDecay.java:52: warning - @param argument "weight" is not a parameter name.
3 warnings


:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 7s
3 actionable tasks: 2 executed, 1 up-to-date
:~/VSCodeProjects/djl$ ./gradlew fJ

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 5s
32 actionable tasks: 32 executed
:~/VSCodeProjects/djl$ ./gradlew compileJava

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 982ms
31 actionable tasks: 31 up-to-date

@aksrajvanshi
Copy link
Contributor

aksrajvanshi commented Mar 25, 2021

Seems to be ok now (see at the end).

Don't now if it is intended, but I got warnings and not errors. Also, couldn't we add this to the tests and generate errors?

TIA

:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

> Task :api:javadoc
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/ElasticNetWeightDecay.java:67: warning - Parameter "lambda1" is documented more than once.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L1WeightDecay.java:49: warning - @param argument "weight" is not a parameter name.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L2WeightDecay.java:52: warning - @param argument "weight" is not a parameter name.
3 warnings
:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 7s
3 actionable tasks: 2 executed, 1 up-to-date
:~/VSCodeProjects/djl$ ./gradlew fJ

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 5s
32 actionable tasks: 32 executed
:~/VSCodeProjects/djl$ ./gradlew compileJava

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 982ms
31 actionable tasks: 31 up-to-date

Sorry, I forgot to mention this. You will have to run ./gradlew :api:checkstyleMain to look for all the errors and fix those. There are still some functions where you have some function parameters and are not defined in the javadoc comments.

@aksrajvanshi
Copy link
Contributor

aksrajvanshi commented Mar 25, 2021

Seems to be ok now (see at the end).
Don't now if it is intended, but I got warnings and not errors. Also, couldn't we add this to the tests and generate errors?
TIA

:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

> Task :api:javadoc
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/ElasticNetWeightDecay.java:67: warning - Parameter "lambda1" is documented more than once.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L1WeightDecay.java:49: warning - @param argument "weight" is not a parameter name.
/home/hmf/VSCodeProjects/djl/api/src/main/java/ai/djl/training/loss/L2WeightDecay.java:52: warning - @param argument "weight" is not a parameter name.
3 warnings
:~/VSCodeProjects/djl$ ./gradlew :api:javadoc

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 7s
3 actionable tasks: 2 executed, 1 up-to-date
:~/VSCodeProjects/djl$ ./gradlew fJ

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 5s
32 actionable tasks: 32 executed
:~/VSCodeProjects/djl$ ./gradlew compileJava

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.7.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 982ms
31 actionable tasks: 31 up-to-date

Sorry, I forgot to mention this. You will have to run ./gradlew :api:checkstyleMain to look for all the errors and fix those. There are still some functions where you have some function parameters and are not defined in the javadoc comments.

Look into these errors.

Screen Shot 2021-03-25 at 11 07 56 AM

@hmf
Copy link
Contributor Author

hmf commented Mar 25, 2021

Thanks. Found it already. Apologies for the mess.

@aksrajvanshi
Copy link
Contributor

aksrajvanshi commented Mar 25, 2021

Thanks. Found it already. Apologies for the mess.

So I see the build failing again. Run the following commands and see if everything works alright without error and commit your changes again.

./gradlew fJ
./gradlew compileJava
./gradlew verifyJava

and finally

./gradlew :api:checkstyleMain

@codecov-io
Copy link

Codecov Report

Merging #788 (450edf1) into master (ea5d7be) will increase coverage by 0.06%.
The diff coverage is 89.06%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #788      +/-   ##
============================================
+ Coverage     70.17%   70.24%   +0.06%     
- Complexity     4930     4955      +25     
============================================
  Files           483      486       +3     
  Lines         21703    21775      +72     
  Branches       2257     2261       +4     
============================================
+ Hits          15231    15295      +64     
- Misses         5272     5279       +7     
- Partials       1200     1201       +1     
Impacted Files Coverage Δ Complexity Δ
api/src/main/java/ai/djl/training/loss/Loss.java 61.70% <30.00%> (-8.57%) 21.00 <3.00> (+3.00) ⬇️
...va/ai/djl/training/loss/ElasticNetWeightDecay.java 100.00% <100.00%> (ø) 8.00 <8.00> (?)
.../main/java/ai/djl/training/loss/L1WeightDecay.java 100.00% <100.00%> (ø) 6.00 <6.00> (?)
.../main/java/ai/djl/training/loss/L2WeightDecay.java 100.00% <100.00%> (ø) 6.00 <6.00> (?)
api/src/main/java/ai/djl/repository/Artifact.java 89.65% <0.00%> (-3.45%) 36.00% <0.00%> (-2.00%)
.../ai/djl/serving/http/ManagementRequestHandler.java 92.17% <0.00%> (-0.75%) 26.00% <0.00%> (+1.00%) ⬇️
...ving/src/main/java/ai/djl/serving/ModelServer.java 48.34% <0.00%> (+0.34%) 11.00% <0.00%> (ø%)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea5d7be...450edf1. Read the comment docs.

@aksrajvanshi aksrajvanshi requested a review from a team March 26, 2021 00:50
Copy link
Contributor

@zachgk zachgk left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks @hmf

@zachgk zachgk merged commit c317884 into deepjavalibrary:master Mar 29, 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.

None yet

4 participants