search

facebookresearch / ClassyVision

An end-to-end PyTorch framework for image and video classification
https://classyvision.ai
MIT License
1.59k stars 277 forks source link

Adding adadelta to the optimzer series #631

Closed 0tist closed 3 years ago

0tist commented 4 years ago

@facebook-github-bot how can i ask someone to kindly review my pull request

0tist commented 4 years ago

@vreis changes done

mannatsingh commented 4 years ago

@0tist thanks for the PR! Looks like master is broken right now, let me fix that first so that your tests can start running. Also, the notebook changes haven't been reverted still :)

0tist commented 4 years ago

@mannatsingh will do

0tist commented 4 years ago

@mannatsingh @vreis cleaned the notebooks

0tist commented 4 years ago

@mannatsingh done the changes you requested, also....i dont understand what you mean by nit: "some comment"

mannatsingh commented 4 years ago

My bad, you can interpret nit: "some comment" as just "some comment" The "nit" just means I'm nitpicking and it's not a huge deal :)

0tist commented 4 years ago

@mannatsingh is there anything left?

mannatsingh commented 4 years ago

Hi @0tist there are a bunch of typos in the PR. I tried fixing them but there were also mismatches in the types. Could you fix all of these issues by making sure the tests pass? If you rebase on the latest master, CircleCI has also started working properly.

0tist commented 4 years ago

Hi @mannatsingh , i couldn't find mismatches in the PR and the typos, would you mind listing a few for reference, so that i can get a hint

0tist commented 4 years ago

hi @mannatsingh Did the required changes, im clueless as to why the tests are not working

mannatsingh commented 3 years ago

@0tist are you able to see the circle CI details? That should take you to https://app.circleci.com/pipelines/github/facebookresearch/ClassyVision/1877/workflows/41ddc452-d0e3-4a7b-8594-388990c1f86d/jobs/3452 with the job details. These are the errors I see - 

ImportError: Failed to import test module: tasks_fine_tuning_task_test
Traceback (most recent call last):
  File "/opt/circleci/.pyenv/versions/3.6.2/lib/python3.6/unittest/loader.py", line 428, in _find_test_path
    module = self._get_module_from_name(name)
  File "/opt/circleci/.pyenv/versions/3.6.2/lib/python3.6/unittest/loader.py", line 369, in _get_module_from_name
    __import__(name)
  File "/home/circleci/ClassyVision/test/tasks_fine_tuning_task_test.py", line 9, in <module>
    from test.generic.config_utils import get_fast_test_task_config
  File "/home/circleci/ClassyVision/test/generic/config_utils.py", line 9, in <module>
    from classy_vision.tasks import build_task
  File "/home/circleci/ClassyVision/classy_vision/tasks/__init__.py", line 72, in <module>
    from .classification_task import ClassificationTask  # isort:skip
  File "/home/circleci/ClassyVision/classy_vision/tasks/classification_task.py", line 38, in <module>
    from classy_vision.optim import (
  File "/home/circleci/ClassyVision/classy_vision/optim/__init__.py", line 95, in <module>
    import_all_modules(FILE_ROOT, "classy_vision.optim")
  File "/home/circleci/ClassyVision/classy_vision/generic/registry_utils.py", line 20, in import_all_modules
    importlib.import_module(module_name)
  File "/opt/circleci/.pyenv/versions/3.6.2/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/circleci/ClassyVision/classy_vision/optim/adadelta.py", line 8, in <module>
    @register_optimzer("adadelta")
NameError: name 'register_optimzer' is not defined

Also, you don't need to wait for the tests to run here, you can just run the test locally on your development machine and make sure they pass.

mannatsingh commented 3 years ago

No updates, closing this PR