Skip to content
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

avoid some conversions to short_digraph when the input is an instance of StaticSparseBackend #39216

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

Conversation

dcoudert
Copy link
Contributor

Many methods convert the input graph to a short_digraph for faster operations. When the input is an instance of StaticSparseBackend, we can directly get the short_digraph and so avoid the extra cost of the conversion.
On the way, we fix some memory leaks as some instances of short_digraph were not freed.

In a follow-up PR, we will do more in distances_all_pairs.pyx, hyperbolicity.pyx and traversals.pyx.

📝 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

Copy link

github-actions bot commented Dec 28, 2024

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

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 1, 2025
    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` when the
input graph is an instance of `StaticSparseBackend`.

Furthermore, we use the new `PairingHeap_of_n_integers` data structure
(sagemath#39046) instead of a `priority_queue` to emulate a max-heap. This is
slightly faster this way and cleaner.


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39217
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 3, 2025
    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` when the
input graph is an instance of `StaticSparseBackend`.

Furthermore, we use the new `PairingHeap_of_n_integers` data structure
(sagemath#39046) instead of a `priority_queue` to emulate a max-heap. This is
slightly faster this way and cleaner.


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39217
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 20, 2025
…nces_all_pairs.pyx`

    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` in
`distances_all_pairs.pyx` when the input graph is an instance of
`StaticSparseBackend`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39223
Reported by: David Coudert
Reviewer(s): Frédéric Chapoton
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 23, 2025
…nces_all_pairs.pyx`

    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` in
`distances_all_pairs.pyx` when the input graph is an instance of
`StaticSparseBackend`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39223
Reported by: David Coudert
Reviewer(s): Frédéric Chapoton
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 25, 2025
…nces_all_pairs.pyx`

    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` in
`distances_all_pairs.pyx` when the input graph is an instance of
`StaticSparseBackend`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39223
Reported by: David Coudert
Reviewer(s): Frédéric Chapoton
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 26, 2025
…nces_all_pairs.pyx`

    
Similarly to sagemath#39216, we avoid the conversion to `short_digraph` in
`distances_all_pairs.pyx` when the input graph is an instance of
`StaticSparseBackend`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39223
Reported by: David Coudert
Reviewer(s): Frédéric Chapoton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant