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

Rename optimizer_config to hyperparameters #10

Closed
wants to merge 3 commits into from

Conversation

vreis
Copy link
Contributor

@vreis vreis commented Sep 23, 2019

Summary:
The name "optimizer_config" is confusing since it does not come from the
config used to build the optimizer. I think hyperparameters makes that purpose
more clear.

Differential Revision: D17517990

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 23, 2019
Summary:
This will make more sense after you review D17515697: I'm hiding ClassyState
from the public API, but in some cases we access state.optimizer, state.model
etc. This diff adds these members to ClassyTask so it can replace the state.
Eventually I want to move these from the state to the task, but for now we
need them in both.

Differential Revision: D17515732

fbshipit-source-id: aea50c472f27bdc99e66a7fe6acd668ecf2fee2e
Summary:
Move things around a bit more to get closer to our goal. Introduce a
ClassyTrainer.train method, which only takes a task in. The state is hidden
from the public API that way. Other parameters are passed in the constructor
instead.

Differential Revision: D17515697

fbshipit-source-id: 952ecd62b45bed41a6ec3b29d0f1e9f26d4a5b8c
Summary:
The name "optimizer_config" is confusing since it does not come from the
config used to build the optimizer. I think parameters makes that purpose
more clear.

Differential Revision: D17517990

fbshipit-source-id: 42370e84de685c4f15e2f2d02eee132919d2243f
facebook-github-bot pushed a commit that referenced this pull request Sep 24, 2019
Summary:
Pull Request resolved: #10

The name "optimizer_config" is confusing since it does not come from the
config used to build the optimizer. I think parameters makes that purpose
more clear.

Reviewed By: mannatsingh

Differential Revision: D17517990

fbshipit-source-id: f2906668a9ea571bc00497b5f898d14755066bd8
facebook-github-bot pushed a commit that referenced this pull request Apr 18, 2020
Summary:
Pull Request resolved: fairinternal/ssl_scaling#10

Pull Request resolved: #477

In order to get rid of ClassyTrainer, we need to move num_dataloader_workers and dataloader_mp_context somewhere else. Move the context to the task and num_workers to ClassyDataset. This also changes the default multiprocessing context to spawn, for consistency. That required changing tests in fblearner to dev-nosan.

This is a breaking change to the configs, and I'll announce it in the group before landing. Will also try to land other breaking changes at the same time to minimize disruption.

Reviewed By: mannatsingh

Differential Revision: D20803692

fbshipit-source-id: f5091bd93b3265271411f6dcd8d995b56c1cd565
vaibhava0 pushed a commit to vaibhava0/ClassyVision that referenced this pull request Jan 19, 2021
Summary:
Pull Request resolved: fairinternal/ssl_scaling#10

Pull Request resolved: facebookresearch#477

In order to get rid of ClassyTrainer, we need to move num_dataloader_workers and dataloader_mp_context somewhere else. Move the context to the task and num_workers to ClassyDataset. This also changes the default multiprocessing context to spawn, for consistency. That required changing tests in fblearner to dev-nosan.

This is a breaking change to the configs, and I'll announce it in the group before landing. Will also try to land other breaking changes at the same time to minimize disruption.

Reviewed By: mannatsingh

Differential Revision: D20803692

fbshipit-source-id: f5091bd93b3265271411f6dcd8d995b56c1cd565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants