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

Fix Mlflow logging | Add possibility of optional calling of "after run" hooks. #1125

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ihoholko
Copy link

@ihoholko ihoholko commented May 5, 2023

Mlflow Logging

Mlflow doesn't support such characters as @. Pull request fixes this by replacing @ with _at_.

MlflowLogger: Invalid metric name: 'val/AR@100'. Names may only contain alphanumerics, underscores (_), dashes (-), periods (.), spaces ( ), and slashes (/) 

after run hooks.

Current pipeline doesn't allow to log artifacts and perform test after train with active hooks. This PR fixes this by adding optinal call_hook_after_run to train and test methods. :

    # start training
    runner.train(call_hook_after_run = False)
    
    runner.load_checkpoint(runner.message_hub.get_info('best_ckpt'))
    
    # start testing
    results = runner.test(call_hook_after_run = False)
    
    
    
    if mlflow.active_run():
        results = {k.replace('@', '_at_'): v for k, v in results.items()}
        mlflow.log_metrics(results)
        
        mlflow.log_artifact(runner.message_hub.get_info('best_ckpt'))
        mlflow.pytorch.log_model(runner.model, 'model')
        
        
    runner.call_hook('after_run')

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@HAOCHENYE
Copy link
Collaborator

Hi! Please sign the CLA

image

@HAOCHENYE
Copy link
Collaborator

Hi! Please follow the guide in contributing.md to fix the lint error

@OpenMMLab-Assistant-004
Copy link

Hi @ihoholko,

We'd like to express our appreciation for your valuable contributions to the mmengine. Your efforts have significantly aided in enhancing the project's quality.
It is our pleasure to invite you to join our community thorugh Discord_Special Interest Group (SIG) channel. This is a great place to share your experiences, discuss ideas, and connect with other like-minded people. To become a part of the SIG channel, send a message to the moderator, OpenMMLab, briefly introduce yourself and mention your open-source contributions in the #introductions channel. Our team will gladly facilitate your entry. We eagerly await your presence. Please follow this link to join us: ​https://discord.gg/UjgXkPWNqA.

If you're on WeChat, we'd also love for you to join our community there. Just add our assistant using the WeChat ID: openmmlabwx. When sending the friend request, remember to include the remark "mmsig + Github ID".

Thanks again for your awesome contribution, and we're excited to have you as part of our community!

@HAOCHENYE
Copy link
Collaborator

Thanks for your contribution, the modification of MLFLow makes sense to me, however, we need to reconsider the modifications for the runner. I believe the core issue lies in when to close the visualizer. It appears that closing it in the LoggerHook.after_run is not a very good approach.

@mmeendez8
Copy link
Contributor

@HAOCHENYE should we create a new PR for fixing the MLFlow logging issue? It is currently a problem in mmdet:
open-mmlab/mmdetection#10258

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

Successfully merging this pull request may close these issues.

5 participants