Open
Conversation
bwatrous
reviewed
Jun 4, 2025
bwatrous
reviewed
Jun 4, 2025
bwatrous
reviewed
Jun 4, 2025
4a212bf to
3f08f03
Compare
bwatrous
previously approved these changes
Jun 4, 2025
aditigaur4
reviewed
Jun 4, 2025
| str: ASCII visualization of the topology. | ||
| """ | ||
|
|
||
| if self.topo_type == TopologyType.BLOCK: |
Collaborator
There was a problem hiding this comment.
When you have extremely big if else blocks-- its always best to condense them into functions.
if self.topo_type == TopologyType.BLOCK:
visualize_block_topology()
else:
# assuming tree
visualize_tree_topology()
Collaborator
There was a problem hiding this comment.
It would also be nice if visualize_topology* functions actually get moved to util since its not really part of core topology logic. You also dont really need any data from the topology class. Just the string and the block size. Thats an optional suggestion though.
3f08f03 to
14d5ba0
Compare
14d5ba0 to
8d018cb
Compare
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.
Add visualization tool for topology cli to create ASCII art to visualize block and tree slurm topologies
visualizefunction toTopologyclassrunfunction now returns the topology string sovisualizefunction can take it in to generate the visualizationExample Block topology visual:
Example Block topology visualization with nodes less than max_block_size
Example Block Topology visual with block with nodes less than min block size
Example Tree topology visual: