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

[WIP] Type hints for NN and param store #2865

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

[WIP] Type hints for NN and param store #2865

wants to merge 6 commits into from

Conversation

kamathhrishi
Copy link
Contributor

This PR continues the effort of adding type hints to the codebase as described in issue #2550. This PR will add type hints to NN and param module. Additionally, it will cover the utils script and some parts of Optim that weren't completed in the previous PR.

@kamathhrishi
Copy link
Contributor Author

@fritzo What exactly are the datatypes parameters? in

def replace_param(self, param_name, new_param, old_param):

@fritzo fritzo added the WIP label Jun 11, 2021
@fritzo
Copy link
Member

fritzo commented Jun 11, 2021

@kamathhrishi I believe the types are

def replace_param(self, param_name: str, new_param: torch.Tensor, old_param: torch.Tensor):

@fritzo
Copy link
Member

fritzo commented Jun 15, 2021

Hi @kamathhrishi no rush, just FYI we switched from travis-ci to github actions so you'll need to merge the dev branch into this for ci to pass.

@kamathhrishi
Copy link
Contributor Author

kamathhrishi commented Jun 15, 2021

Hi @kamathhrishi no rush, just FYI we switched from travis-ci to github actions so you'll need to merge the dev branch into this for ci to pass.

Thanks for letting me know. Not sure why Github doesn't notify here when my fork is lagging behind. Also I haven't formatted my code yet so it will continue to fail.

@eb8680
Copy link
Member

eb8680 commented Aug 3, 2021

@kamathhrishi is this ready for review? No rush, just don't want you blocked - if so, can you run make lint locally and fix the linter errors?

@kamathhrishi
Copy link
Contributor Author

@eb8680 I will come back to this in September. Till then I will close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants