Merged
Conversation
… local mode Now also supported: - Callable class instances (implementing `__call__`) - Classes that implement `__call__ ` but are not yet instantiated (become a ray actor, yet to be tested)
It is directly related to the evaluation on the block worker, therefore it is a better fit.
giuse
requested changes
Oct 27, 2025
Owner
giuse
left a comment
There was a problem hiding this comment.
Thanks for bringing DiBB up to speed! I really like this. There's one file that should not be committed (and added to .gitignore), and one stylistic change I would rather roll back (switching all python strings to double quotes, I use single quotes to distinguish symbols, which are otherwise missing from python), otherwise all good and very happy with this! :)
When using DiBB in local mode, i.e. we only have one ip address in the Ray cluster, it does not matter which IP it is. Moreover, when using DiBB on a SLURM cluster node, Ray may not show 127.0.0.1 as the ip adress.
…ndling of string literals. Refactored string quotes in various files to maintain consistency.
…preventing unwanted changes during commits. Updated test assertion formatting for improved readability.
…on of Ruff formatting.
Collaborator
Author
|
@giuse I addressed your comments and changed three additional things:
index e90cb84..d8085b7 100644
@@ -118,7 +118,7 @@ class DiBB:
assert sum(self.block_sizes) == self.ndims, (
"Sum of block sizes must sum up to number of dimensions "
- + f"(tried {len(self.blocks)} blocks totaling {sum(self.blocks)} "
+ + f"(tried {len(self.block_sizes)} blocks totaling {sum(self.block_sizes)} "
+ f"dimensions with {self.ndims} parameters)"
)
index d8085b7..8315e42 100644
@@ -143,9 +143,6 @@ class DiBB:
ip_addresses = [node["NodeManagerAddress"] for node in ray.nodes()]
if len(ip_addresses) == 1:
# Local mode
- assert ip_addresses[0] == "127.0.0.1", (
- "Unexpected error: Running on a single machine which is not 127.0.0.1"
- )
index c3ec867..6c4b727 100644
@@ -320,6 +320,10 @@ class BlockWorker:
if self.verbose:
print(f"Resuming block {self.block_id}")
return done
+
+ def __repr__(self):
+ """Customized prefix for Ray actor logs"""
+ return f"BlockWorker [block_id={self.block_id}]"Can you have a look at the PR again and give further feedback if needed? |
Owner
|
Great work, thank you so much @rolshoven ! :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes the following changes: