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

inconsistent behavior of .copy() on immutable graphs #35592

Open
2 tasks done
maxale opened this issue May 1, 2023 · 7 comments
Open
2 tasks done

inconsistent behavior of .copy() on immutable graphs #35592

maxale opened this issue May 1, 2023 · 7 comments
Labels

Comments

@maxale
Copy link
Contributor

maxale commented May 1, 2023

Is there an existing issue for this?

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.

Did you read the documentation and troubleshoot guide?

  • I have read the documentation and troubleshoot guide

Environment

- **OS**: Ubuntu 22.04
- **Sage Version**: 9.8.rc0

Steps To Reproduce

Function f(H) below print the immutability status of a given graph H and that of H.copy():

def f(H):
    print('H:', H.is_immutable(),flush=True)
    print('H.copy():', H.copy().is_immutable(),flush=True)
    

G = Graph(immutable=True)
f(G)

import multiprocessing
multiprocessing.Pool().imap_unordered(f,[G])

Expected Behavior

Whether we use multiprocessing or not, H.copy() should inherit the immutability of H.

Actual Behavior

The code output is:

H: True
H.copy(): True
<multiprocessing.pool.IMapUnorderedIterator object at 0x7f05e6ada680>
H: True
H.copy(): False

First, we create an immutable graph G. When we call f(G) directly, the created copy of it is immutable (as expected). However, when we call the same function using via multiprocessing, f reports the the input graph is still immutable (which is good), while its copy somehow becomes not immutable (which is bad).

Additional Information

No response

@maxale maxale added the t: bug label May 1, 2023
@mkoeppe
Copy link
Member

mkoeppe commented May 1, 2023

@nbruin
Copy link
Contributor

nbruin commented May 1, 2023

Actually, an immutable "copy" of an immutable graph could actually just be a reference to the original. It's immutable after all! The only reason to make a copy of an immutable object is to get a mutable copy. I'm pretty sure this is the rationale behind the behaviour. It is consistent with, for instance, vectors:

sage: v=vector([1,2,3],immutable=True)
sage: v.is_immutable()
True
sage: w=copy(v)
sage: w.is_immutable()
False

@maxale
Copy link
Contributor Author

maxale commented May 1, 2023

I'm fine with whatever outcome of .copy() (immutable or not), but this bugreport about its behavior being inconsistent across the two calls to f(G) on the same immutable graph G: in one (direct) call it creates an immutable copy, while in the other (via multiprocessing) it creates a mutable copy.

@maxale
Copy link
Contributor Author

maxale commented May 2, 2023

Btw, #34598 is another issue when using multiprocessing breaks behavior of Sage objects. I suspect it having the same origin as the current one.

@nbruin
Copy link
Contributor

nbruin commented May 2, 2023

A workaround may be to use copy(G). By the looks of it, this is consistent.

It looks like the immutable handling on Graph predates the more standardized approaches: set_immutable doesn't exist on graphs, for one thing, and one can very well imagine this to be useful (build a graph through various mutations and then fix them to get an immutable and hopefully hashable object).

If you change the applied function to

sage: def f(H):
....:     print((H).__dict__,flush=True)

you can see that the reproduction through multiprocessing has a different backend defined. Looking at Graph.copy you'll see that this can have various consequences. It also shows that copy has some other function than to just get a copy of the existing (mutable) object. In fact the documentation has a warning:

   Warning:

     Please use this method only if you need to copy but change the
     underlying data structure or weightedness. Otherwise simply do
     "copy(g)" instead of "g.copy()".

@maxale
Copy link
Contributor Author

maxale commented May 3, 2023

If you change the applied function to

sage: def f(H):
....:     print((H).__dict__,flush=True)

you can see that the reproduction through multiprocessing has a different backend defined.

Thank you for the insight. As I view it now, the current issue is in fact a combination of two rather unrelated ones:

  • multiprocessing causes the change in the backend for graph objects passed over to a parallel function: from sage.graphs.base.static_sparse_backend.StaticSparseBackend to sage.graphs.base.sparse_graph.SparseGraphBackend
  • the .copy() method has inconsistent behavior across the two backends.

Personally I care more about the first issue as I see some other buggy things happening around it, while .copy() behavior is rather a minor issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants