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

Move hivemind.Server from init, streamline imports #441

Merged
merged 9 commits into from
Jan 9, 2022
Merged

Conversation

mryab
Copy link
Member

@mryab mryab commented Jan 8, 2022

This is a chore PR that mainly moves hivemind.Server and related functions into a separate file to keep server/__init__.py simple and avoid exposing unintended methods at the top level of the library.

Also, I replaced broad import hivemind statements with more specific ones in relevant parts of the project to narrow down their scope and avoid possible circular imports in the future.

Finally, I moved generate_uids_from_pattern to server.py, since the refactoring from the first step exposed a circular dependency in the module. This can be improved in the future via better encapsulation of ExpertUID (right now its constants are used all over hivemind.moe); for now, I believe the solution with a private function is fine.

@codecov
Copy link

codecov bot commented Jan 8, 2022

Codecov Report

Merging #441 (f3f2ebb) into master (c868989) will increase coverage by 0.24%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
+ Coverage   83.75%   83.99%   +0.24%     
==========================================
  Files          77       78       +1     
  Lines        7927     7928       +1     
==========================================
+ Hits         6639     6659      +20     
+ Misses       1288     1269      -19     
Impacted Files Coverage Δ
hivemind/hivemind_cli/run_server.py 0.00% <0.00%> (ø)
hivemind/moe/server/server.py 81.57% <81.57%> (ø)
hivemind/moe/__init__.py 100.00% <100.00%> (ø)
hivemind/moe/client/moe.py 92.82% <100.00%> (ø)
hivemind/moe/server/__init__.py 100.00% <100.00%> (+11.68%) ⬆️
hivemind/moe/server/expert_uid.py 100.00% <100.00%> (+30.35%) ⬆️
hivemind/averaging/averager.py 87.65% <0.00%> (+0.72%) ⬆️
hivemind/utils/mpfuture.py 95.00% <0.00%> (+0.90%) ⬆️
hivemind/optim/progress_tracker.py 98.90% <0.00%> (+1.09%) ⬆️
hivemind/dht/node.py 92.63% <0.00%> (+1.18%) ⬆️
... and 1 more

@mryab mryab requested a review from justheuristic January 8, 2022 15:01
A working server does 3 things:
- processes incoming forward/backward requests via Runtime (created by the server)
- publishes updates to expert status every :update_period: seconds
- follows orders from HivemindController - if it exists
Copy link
Member Author

Choose a reason for hiding this comment

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

image

I guess this was something that we planned to implement, but never did. Stumbled upon it by pure chance

Copy link
Member

@justheuristic justheuristic Jan 9, 2022

Choose a reason for hiding this comment

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

Curiously, the doc statement is still technically correct :D

class VorpalSword(SwordBase):
    """A special sword used to kill Jabberwocky - if it exists"""

As far as I remember, this was my wishful thinking. The idea was for HivemindController to be a class that can change server config over time: add or remove experts, change hyperparameters, stuff like that...

Makes me think how many of these lost dreams we still have in other docstrings :)

@mryab mryab merged commit d29101e into master Jan 9, 2022
@mryab mryab deleted the refactor_server branch January 9, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants