Skip to content

Fixed ClearMLLogger to retrieve current task before trying to create a new one.#2344

Merged
vfdev-5 merged 8 commits intopytorch:masterfrom
H4dr1en:master
Dec 13, 2021
Merged

Fixed ClearMLLogger to retrieve current task before trying to create a new one.#2344
vfdev-5 merged 8 commits intopytorch:masterfrom
H4dr1en:master

Conversation

@H4dr1en
Copy link
Contributor

@H4dr1en H4dr1en commented Dec 10, 2021

Fixes #2343

Description: ClearMLLogger tries to retrieve current task before trying to create a new one.

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: contrib Contrib module label Dec 10, 2021
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 10, 2021

@H4dr1en thanks for the PR ! Do you think we can write a distributed test for checking that everything is working ?

@H4dr1en
Copy link
Contributor Author

H4dr1en commented Dec 10, 2021

@vfdev-5 This might be very tricky to test: we want to make sure that the communication between the subprocesses and the main process works well, so that clearml can log successfully in the clearml-server. I have now idea how to test that :D

Especially since there is no error to catch, this happens silently in the background

@H4dr1en H4dr1en requested a review from vfdev-5 December 10, 2021 14:40
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@H4dr1en thanks a lot for the PR !
For the tests I agree that this could be a bit tricky to write and there is no error to catch.
Let's merge this like that and if possible to add a test we can do that later in a follow-up PR.
I still would like to hear from ClearML team.
@bmartinn could you please review as well. Thanks !

EDIT: There is a lint to fix and TPU tests are unrelated

@vfdev-5 vfdev-5 enabled auto-merge (squash) December 13, 2021 11:04
@vfdev-5 vfdev-5 merged commit 4ddcf29 into pytorch:master Dec 13, 2021
Ishan-Kumar2 pushed a commit to Ishan-Kumar2/ignite that referenced this pull request Dec 26, 2021
* fix pytorch#2343

* update docstring

* add comment

* fix docstrings

* remove trailing whitespace

Co-authored-by: vfdev <[email protected]>
@vfdev-5 vfdev-5 changed the title Fix #2343 Fixed ClearMLLogger to retrieve current task before trying to create a new one. Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: contrib Contrib module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClearrMLLogger freezes logging in distributed env

2 participants