search

V2AI / Det3D

World's first general purpose 3D object detection codebse.
https://arxiv.org/abs/1908.09492
Apache License 2.0
1.49k stars 298 forks source link

Support fastai_optimizer for all lr schedulers #34

Closed s-ryosky closed 4 years ago

s-ryosky commented 4 years ago

ref: #21

poodarchu commented 4 years ago

Is it correct to use optimizer_config.type rather than optimizer_config.TYPE? It's mismatch with config file.

s-ryosky commented 4 years ago

I have used ".type" rather than ".TYPE" because you have set "type" in config files. It depends the way to specify the attribute.

https://github.com/poodarchu/Det3D/blob/7b28766a8fb61f223ecaae012c2447d8ad2138f3/examples/cbgs/configs/nusc_all_vfev3_spmiddleresnetfhd_rpn2_mghead_syncbn.py#L344-L348

poodarchu commented 4 years ago

please keep it as Upper Case to indicate that thoese options are corresponding to config files.

s-ryosky commented 4 years ago

Do you mean to change option names in config files to uppercase?

If so, then may I change all of them to uppercase? Or may I change only option names which are related to optimizer config to uppercase?

Now all option names in config files are written as lowercase and In your implementation the most of options are used as follows: https://github.com/poodarchu/Det3D/blob/56402d4761a5b73acd23080f537599b0888cce07/det3d/torchie/apis/train.py#L229-L230

poodarchu commented 4 years ago

YES, UPPER CASE is more suitable here

s-ryosky commented 4 years ago

Fix.

It keeps option names of fastai_optimizer as uppercase. The others are lowercase.

poodarchu commented 4 years ago

please fix commit history, unrelated files are included in this merge.

s-ryosky commented 4 years ago

sorry, I fixed it.

s-ryosky commented 4 years ago

any feedback?