Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed issue the caused random element in for Tuples to stall by adding unrank method. #39626

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Noel-Roemmele
Copy link
Contributor

Fixes #39534. There was an issue in Tuples causing random element to stall. It was stalling because Tuples did not have an efficient method of getting the i-th element without iterating through the entire set. To fix this an unrank method was added to Tuples. This method was based on the unrank method found in src\sage\categories\finite_enumerated_sets.py. This unrank method creates each tuple in a similar way to how numbers in some base are created. Also added documentation, an example, and various tests to make sure the function is working correctly.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@Noel-Roemmele
Copy link
Contributor Author

One issue that I haven't yet solved is if the index is negative. If the index is negative it seems to default back to the original behavior of the function. So if we try to do Tuples(range(3), 30)[-10^12] not only does it not throw an error as expected it also stalls as before the change. I'm unsure of how to fix this.

Copy link

github-actions bot commented Mar 3, 2025

Documentation preview for this PR (built with commit 36652ab; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@user202729
Copy link
Contributor

user202729 commented Mar 3, 2025

One issue that I haven't yet solved is if the index is negative. If the index is negative it seems to default back to the original behavior of the function. So if we try to do Tuples(range(3), 30)[-10^12] not only does it not throw an error as expected it also stalls as before the change. I'm unsure of how to fix this.

Caused by /src/sage/categories/enumerated_sets.py's implementation of __getitem__ method.

Since the current implementation appears to be explicitly worse than return self.unrank(cardinality() + i) (if unrank and cardinality overloaded then is explicitly worse, otherwise worst case is 2× slower but only O(1) memory used) I'd recommend just changing it at the source.


Also I recommend testing that unrank is consistent with iteration order, and when the base set is infinite/uncountable, and calling .unrank() directly and/or adding # indirect doctest.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.random_element() stalls for Tuples
2 participants