Opened 4 years ago

Closed 3 years ago

#24589 closed defect (fixed)

Pickling matrices over GF(2) does not preserve their immutability

Reported by: nbruin Owned by:
Priority: minor Milestone: sage-8.4
Component: pickling Keywords:
Cc: embray, jdemeyer, slelievre Merged in:
Authors: Travis Scrimshaw Reviewers: Nils Bruin
Report Upstream: N/A Work issues:
Branch: b93cd49 (Commits, GitHub, GitLab) Commit: b93cd4963b399542d265c2b803dc7ad391f37912
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

sage: A = matrix(GF(2), [[1, 0], [0, 1]])
sage: A.set_immutable()
sage: loads(dumps(A)).is_immutable()

from explain_pickle(dumps(A)) it looks like the pickling routine for sage.matrix.matrix_mod2_dense is just a little too naive. For several other base rings immutability is preserved.

Reported on

Change History (7)

comment:1 Changed 3 years ago by slelievre

  • Cc embray jdemeyer slelievre added
  • Description modified (diff)

comment:2 Changed 3 years ago by embray

  • Milestone changed from sage-8.2 to sage-8.4

It's probably a simple fix in case anyone would like to try to do it for Sage 8.4 (which I believe will be accepting new fixes for ~1 week).

comment:3 Changed 3 years ago by nbruin

I'm afraid it could be a bit of work: You need to store a new value in the pickle, so this possibly needs a new format for pickling these matrices. Some care needs to be taken that old pickles are still read correctly. When I made the ticket I did take a look if I could fix it easily and at the time I concluded it was a bit more work/took a little more expertise than I had available.

comment:4 Changed 3 years ago by tscrim

  • Authors set to Travis Scrimshaw
  • Branch set to public/pickling/immutability_dense_mod2-24589
  • Commit set to b93cd4963b399542d265c2b803dc7ad391f37912
  • Status changed from new to needs_review

It should just be a matter of appending on another optional value immutable=True to the unpickle_matrix_mod2_dense_v1 (or maybe call it v2 with an register_unpickle_override) and just add the extra input of the _is_immutable to the corresponding __reduce__. So I did that on this branch.

New commits:

b93cd49Adding immutable to the pickling for GF(2) dense matrices.

comment:5 Changed 3 years ago by nbruin

  • Reviewers set to Nils Bruin
  • Status changed from needs_review to positive_review

Thanks. That's what I roughly had in mind. It's good you know the details for how to accomplish it.

comment:6 Changed 3 years ago by tscrim

Thanks for the review.

comment:7 Changed 3 years ago by vbraun

  • Branch changed from public/pickling/immutability_dense_mod2-24589 to b93cd4963b399542d265c2b803dc7ad391f37912
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.