-
Notifications
You must be signed in to change notification settings - Fork 505
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
Support libp2p relays for NAT traversal #186
Changes from 2 commits
5ee93df
d641b9b
d99d710
5e1c9fc
a8f20de
a39e544
59f465f
6d8322e
43b1997
434b630
dbf504b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,8 @@ def __init__( | |
load_in_8bit: Optional[bool] = None, | ||
tensor_parallel_devices: Optional[Sequence[torch.device]] = None, | ||
skip_reachability_check: bool = False, | ||
use_relay: bool = True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's best to either remove default argument values of to remove these arguments completely: we might forget to change defaults here in the future, and required values will be passed to kwargs anyway There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point but I think we should keep it, since the convention in Petals is that all But all you said would have applied if the defaults here matched with hivemind. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will agree with @borzunov here. Here are my arguments:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point about explicitly indicating arguments for creation of Server, though it somewhat contradicts the existence of **kwargs in init. My primary concern is that we should strive to have consistent defaults across different locations: one way to do this in an error-proof way would be to declare a common constant with the default value and use it in both locations. Besides, petals-cli is a part of Petals, so these files belong to the same library There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would need to create constants for all defaults in this case (tens of them). I think this is a more general problem that should be addressed outside of this PR (maybe we should use smth like reflection). |
||
use_auto_relay: bool = True, | ||
**kwargs, | ||
): | ||
"""Create a server with one or more bloom blocks. See run_server.py for documentation.""" | ||
|
@@ -117,7 +119,14 @@ def __init__( | |
) | ||
self.module_uids = [f"{self.prefix}.{block_index}" for block_index in range(self.block_config.n_layer)] | ||
|
||
self.dht = DHT(initial_peers=initial_peers, start=True, num_workers=self.block_config.n_layer, **kwargs) | ||
self.dht = DHT( | ||
initial_peers=initial_peers, | ||
start=True, | ||
num_workers=self.block_config.n_layer, | ||
use_relay=use_relay, | ||
use_auto_relay=use_auto_relay, | ||
**kwargs, | ||
) | ||
visible_maddrs_str = [str(a) for a in self.dht.get_visible_maddrs()] | ||
if initial_peers == PUBLIC_INITIAL_PEERS: | ||
logger.info(f"Connecting to the public swarm, peer_id = {self.dht.peer_id}") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.