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.

Sign up for GitHub

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

Jump to bottom

Add valohai.distributed with config parsing #88

Merged
merged 9 commits into from
Jun 2, 2022
Merged

Add valohai.distributed with config parsing #88

merged 9 commits into from
Jun 2, 2022

Conversation

ruksi
Copy link
Member

@ruksi ruksi commented May 25, 2022 •
edited
Loading

Fixes #87

TODO:

  • add valohai.distributed to provide common helpers for reading Valohai distributed task configuration
  • add a valohai.distributed section to README

Also includes other small improvements here and there.

@ruksi ruksi force-pushed the distributed-config branch 3 times, most recently from 17c34cb to 7ba3457 Compare May 25, 2022 12:46
@ruksi ruksi marked this pull request as ready for review May 25, 2022 12:49
@ruksi ruksi requested a review from akx May 25, 2022 12:49
akx
akx requested changes May 27, 2022
View reviewed changes
Copy link
Member

@akx akx left a comment

Choose a reason for hiding this comment

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

Smol change requests, but looking pretty neat!

valohai/internals/distributed_config/member.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/network.py Outdated Show resolved Hide resolved
valohai/distributed.py Show resolved Hide resolved
@ruksi ruksi force-pushed the distributed-config branch 3 times, most recently from 6c91188 to fc439dc Compare May 30, 2022 09:26
@ruksi
Copy link
Member Author

ruksi commented May 30, 2022 •
edited
Loading

  • Removed Network structure and moved those properties one level up.
  • Made ranking work deterministically with non-integer member ids and integers that don't start from zero.
  • Updated README to reflect these.

@ruksi ruksi requested a review from akx May 30, 2022 09:31
@ruksi ruksi force-pushed the distributed-config branch 2 times, most recently from 802cd36 to 63d23fe Compare May 30, 2022 09:46
akx
akx requested changes May 30, 2022
View reviewed changes
valohai/internals/distributed_config/member.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/utils.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/utils.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/utils.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/utils.py Show resolved Hide resolved
valohai/internals/distributed_config/utils.py Outdated Show resolved Hide resolved
valohai/distributed.py Outdated Show resolved Hide resolved
valohai/internals/distributed_config/member.py Outdated Show resolved Hide resolved
valohai/distributed.py Outdated Show resolved Hide resolved
valohai/distributed.py Outdated Show resolved Hide resolved
@ruksi ruksi force-pushed the distributed-config branch 2 times, most recently from 5df2c96 to 4aecbc0 Compare June 1, 2022 06:49
@ruksi
Add distributed config "dataclasses"
83b0ae8 
Can't use the actual `dataclasses` as those were introduced in 3.7 and `valohai-utils` currently supports 3.6
@ruksi ruksi force-pushed the distributed-config branch 2 times, most recently from d72eada to f327d0b Compare June 1, 2022 06:59
@ruksi
Copy link
Member Author

ruksi commented Jun 1, 2022

  • yeet MASTER_MEMBER_ID and expect the master always to simply be rank 0
  • rename self() to me() so it won't clash so much with the usual self
  • apply the suggested refactorings

@ruksi ruksi requested a review from akx June 1, 2022 07:04
akx
akx requested changes Jun 1, 2022
View reviewed changes
Copy link
Member

@akx akx left a comment

Choose a reason for hiding this comment

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

One teensy-weensy change request left 🆗

valohai/distributed.py Show resolved Hide resolved
@ruksi ruksi force-pushed the distributed-config branch from b1ffe3e to 648650a Compare June 1, 2022 09:35
@ruksi ruksi force-pushed the distributed-config branch from 648650a to b648e09 Compare June 1, 2022 09:36
@ruksi
Copy link
Member Author

ruksi commented Jun 1, 2022

never failing try/except added

@ruksi ruksi requested a review from akx June 1, 2022 09:38
akx
akx approved these changes Jun 2, 2022
View reviewed changes
Copy link
Member

@akx akx left a comment

Choose a reason for hiding this comment

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

:shipi

@akx akx merged commit 069fc50 into master Jun 2, 2022
@akx akx deleted the distributed-config branch June 2, 2022 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akx akx akx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add distributed.json parsing
2 participants
@ruksi @akx