BUG: add check for isolated nodes in degree_sequence_tree
#8235
Conversation
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
dschult
approved these changes
Sep 6, 2025
Member
dschult
left a comment
dschult
left a comment
There was a problem hiding this comment.
This is great -- the addition of a missing useful utility for the tree suite of functions.
Thanks!
(I have one minor suggestion, but only if you prefer it -- and feel free to change the name choices to something else. :)
Co-authored-by: Dan Schult <dschult@colgate.edu>
Member
Author
|
Thanks Dan, your suggestion improves readability. This PR still needs a label by the way! :) |
MridulS
reviewed
Sep 7, 2025
Comment on lines
+192
to
+193
| t(nx.random_powerlaw_tree, 5, seed=seed, tries=5000) | ||
| t(nx.random_powerlaw_tree_sequence, 5, seed=seed, tries=5000) |
Member
There was a problem hiding this comment.
Any particular reason for changing the n here?
Member
Author
There was a problem hiding this comment.
These tests exceed the maximum number of tries with the old values now that invalid sequences are filtered more thoroughly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #8227.
This PR adds the check suggested in the linked issue for zero-degree nodes, which cannot appear in a valid degree sequence.
To do so, a utility function
is_valid_tree_degree_sequencehas been added, which returns whether a degree sequence follows two basic rules: the number of nodes must be equal to one more than the number of edges, and there cannot be any negative degrees in the degree sequence unless the degree sequence represents the trivial graph. It also gives a string representing which of the two checks was failed, to simplify error reporting downstream; I wasn't sure whether returning the string was worth doing; perhaps downstream functions using this utility function should just raise with a generic"invalid degree sequence for tree"message.This change affects
random_powerlaw_tree_sequencein that it now becomes (significantly?) less likely to yield a "valid" sequence in the given number oftries.