Skip to content

101 no regularization grad - #104

Open
wsdewitt wants to merge 12 commits into
mainfrom
101-no-regularization-grad
Open

101 no regularization grad#104
wsdewitt wants to merge 12 commits into
mainfrom
101-no-regularization-grad

Conversation

@wsdewitt

@wsdewitt wsdewitt commented Feb 23, 2021

Copy link
Copy Markdown
Contributor

Description

Make model.FullyConnected.regularization_loss() differentiable.

Closes #101

Tests

tests/test_model.py

Checklist:

  • The code uses informative and accurate variable and function names
  • The functionality is factored out into functions and methods with logical interfaces
  • Comments are up to date, document intent, and there are no commented-out code blocks
  • Commenting and/or documentation is sufficient for others to be able to understand intent and implementation
  • TODOs have been eliminated from the code
  • The corresponding issue number (e.g. #278) has been searched for in the code to find relevant notes
  • Documentation has been redeployed

@wsdewitt
wsdewitt requested review from matsen and zorian15 February 23, 2021 17:44
@wsdewitt

wsdewitt commented Feb 23, 2021

Copy link
Copy Markdown
Contributor Author

My current challenge is that loss.backward() returns None in my new test module, so I'm having trouble validating the gradient.

@wsdewitt

Copy link
Copy Markdown
Contributor Author

Ok I think this is ready for review

@zorian15 zorian15 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sick, looks good -- I don't have any complaints.

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.

No grad for regularization loss

3 participants