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

Unpickling a graph can change the backend #38900

Closed
2 tasks done
ed359 opened this issue Oct 31, 2024 · 2 comments · Fixed by #38919
Closed
2 tasks done

Unpickling a graph can change the backend #38900

ed359 opened this issue Oct 31, 2024 · 2 comments · Fixed by #38919

Comments

@ed359
Copy link
Contributor

ed359 commented Oct 31, 2024

Steps To Reproduce

In a standard install of sagemath 10.4, run the following code.

G = Graph(1, data_structure="dense")
H = loads(dumps(G))

print(G == H) # True
print(isinstance(G._backend, sage.graphs.base.dense_graph.DenseGraphBackend)) # True
print(isinstance(H._backend, sage.graphs.base.sparse_graph.SparseGraphBackend)) # True

On my system, I get

True
True
True

Expected Behavior

The backend data structure of a graph is preserved upon pickling and unpickling.

Actual Behavior

The backend is not preserved and falls back to the default (sparse) backend.

Additional Information

This is a bug, as I want to use process-level parallelism in some loops that generate graphs. With standard tools such as joblib, multiprocessing, concurrent.futures, this requires some pickling and unpickling. But my code needs the backends of the graphs to be the DenseGraph implementation and this bug prevents me from using these tools.

Environment

  • OS: MacOS Sequoia 15.0.1
  • Sage Version: 10.4 (binary distribution from 3-manifolds project)

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@ed359 ed359 added the t: bug label Oct 31, 2024
@maxale
Copy link
Contributor

maxale commented Nov 1, 2024

Related #35592

@dcoudert
Copy link
Contributor

dcoudert commented Nov 3, 2024

See #38919 for a possible fix.

vbraun pushed a commit to vbraun/sage that referenced this issue Nov 6, 2024
    
Fix sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 7, 2024
    
Fix sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 8, 2024
    
Fix sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 9, 2024
    
Fixes sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 13, 2024
    
Fixes sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Nov 14, 2024
    
Fixes sagemath#38900.

The data structure of the graph was not stored. We add it to the stored
data.

### 📝 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.
- [x] 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#38919
Reported by: David Coudert
Reviewer(s): Travis Scrimshaw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants