Megvii-BaseDetection / YOLOX

YOLOX is a high-performance anchor-free YOLO, exceeding yolov3~v5 with MegEngine, ONNX, TensorRT, ncnn, and OpenVINO supported. Documentation: https://yolox.readthedocs.io/
Apache License 2.0
9.35k stars 2.19k forks source link

mlflow logging integration with yolox training #1773

Closed Im-Himanshu closed 2 months ago

Im-Himanshu commented 4 months ago

Needed integration of yolox to log experiments with mlflow, the pull request provide additional option in -l --logger argument to log output in "mlflow".

Requires an environment file (.env) in the root folder of the projects.

required additional dependency of mlflow and python-dotenv failing which error is raised if logger is set to mlflow.

Im-Himanshu commented 4 months ago

Tested the logging on to data bricks, following logs are available for all the runs.

Logged params

image

logged metrices

image

logged artifacts

image

Im-Himanshu commented 4 months ago

@FateScript Requesting you to please review the pull request.

FateScript commented 4 months ago

Any update? @Im-Himanshu

Im-Himanshu commented 3 months ago

Any update? @Im-Himanshu

@FateScript Excuse me for the delayed response, I have pushed new commits to implement all the suggestions. Kindly review.

Im-Himanshu commented 3 months ago

Any update? @Im-Himanshu

@FateScript Excuse me for the delayed response, I have pushed new commits to implement all the suggestions. Kindly review.

@FateScript Gentle Reminder to Please check and merge the request.

FateScript commented 2 months ago

@Im-Himanshu Also please don't forget to lint your code.

Im-Himanshu commented 2 months ago

@Im-Himanshu Also please don't forget to lint your code.

Linted the code, the only major issue in lint, is the import statement which has to be done inside class (same as being done in wandb logger) because this is an optional feature. image

Im-Himanshu commented 2 months ago

@FateScript @Cloudhax23 completed all the suggestion, please check.

Im-Himanshu commented 2 months ago

@FateScript Removed the additional logger and linted the code again to remove build process error.

FateScript commented 2 months ago

@Im-Himanshu please isort your code(isort -rc), see here for more details.

Im-Himanshu commented 2 months ago

@Im-Himanshu please isort your code(isort -rc), see here for more details.

@FateScript in trainer.py and mlflow_logger.py, isort has transferred the import datetime and other default python package import at the end but it was not the case in original code so as of now I have manually shifted them to the top, please see if it is correct or not.

I think my isort setting are not matched to the project setting, if there are any settings of isort that I can follow for this project, I can rerun the isort or otherwise the sort that I have done now should be correct based on my intuition.

FateScript commented 2 months ago

@Im-Himanshu please isort your code(isort -rc), see here for more details.

@FateScript in trainer.py and mlflow_logger.py, isort has transferred the import datetime and other default python package import at the end but it was not the case in original code so as of now I have manually shifted them to the top, please see if it is correct or not.

I think my isort setting are not matched to the project setting, if there are any settings of isort that I can follow for this project, I can rerun the isort or otherwise the sort that I have done now should be correct based on my intuition.

If you instal the correct isort version(4.3.21 here), everything will be ok. BTW, be sure of the format check here passed before you commit.

Im-Himanshu commented 2 months ago

@Im-Himanshu please isort your code(isort -rc), see here for more details.

@FateScript in trainer.py and mlflow_logger.py, isort has transferred the import datetime and other default python package import at the end but it was not the case in original code so as of now I have manually shifted them to the top, please see if it is correct or not. I think my isort setting are not matched to the project setting, if there are any settings of isort that I can follow for this project, I can rerun the isort or otherwise the sort that I have done now should be correct based on my intuition.

If you instal the correct isort version(4.3.21 here), everything will be ok. BTW, be sure of the format check here passed before you commit.

@FateScript My isort version was not as required by the library, hence the previous errors. Anyway I have removed the given pylint and isort errors. It should pass all the checks this time.

FateScript commented 2 months ago

LGTM.