Opened 2 years ago
Closed 17 months ago
#27315 closed enhancement (fixed)
Minor optimizations to libsingular polynomials
Reported by:  mmezzarobba  Owned by:  

Priority:  minor  Milestone:  sage9.1 
Component:  basic arithmetic  Keywords:  
Cc:  Merged in:  
Authors:  Marc Mezzarobba  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  41fe6dd (Commits, GitHub, GitLab)  Commit:  41fe6dd7965ee6867fc8c62dd1a52ee98f5de383 
Dependencies:  Stopgaps: 
Description
Change History (20)
comment:1 Changed 2 years ago by
 Branch set to u/mmezzarobba/mpoly_minor_opt
 Commit set to b1f807e5cbef7a401fcc5b929e2355e1414374a4
 Dependencies set to #26067
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit changed from b1f807e5cbef7a401fcc5b929e2355e1414374a4 to 47a457ce6a626b60a51dda89f8af1e5b9706a5f3
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
47a457c  #27315 minor optimizations to libsingular mpolys

comment:3 Changed 2 years ago by
 Milestone changed from sage8.7 to sage8.8
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:4 Changed 2 years ago by
 Milestone changed from sage8.8 to sage8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:5 Changed 19 months ago by
 Commit changed from 47a457ce6a626b60a51dda89f8af1e5b9706a5f3 to 35e9cad932c1c39e84244ed4114a9c527921c098
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
35e9cad  #27315 minor optimizations to libsingular mpolys

comment:7 followup: ↓ 8 Changed 19 months ago by
not convinced by the use of "isinstance" here:
 if self._parent._base.is_finite() and self._parent._base.characteristic() > 1<<29: + if n_GetChar(r) > 1<<29 and isinstance(self._parent._base, FiniteField):
comment:8 in reply to: ↑ 7 Changed 19 months ago by
 Status changed from needs_review to needs_work
Replying to chapoton:
not convinced by the use of "isinstance" here:
Ouch—I don't remember at all why I made this change, but I agree that it looks strange. Thank you.
comment:9 Changed 18 months ago by
 Commit changed from 35e9cad932c1c39e84244ed4114a9c527921c098 to 85e31dc66134b0bdb2637024fb492357ab820035
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
85e31dc  #27315 minor optimizations to libsingular mpolys

comment:10 Changed 18 months ago by
 Status changed from needs_work to needs_review
Ok, I removed the redundant test, let's see if the patchbot complains.
comment:11 Changed 18 months ago by
 Milestone changed from sage8.9 to sage9.1
Ticket retargeted after milestone closed
comment:12 Changed 17 months ago by
 Status changed from needs_review to needs_work
red branch => needs work
comment:13 Changed 17 months ago by
 Commit changed from 85e31dc66134b0bdb2637024fb492357ab820035 to f8cd16194a7ee7d4adf482cb07b2b6c79d07f812
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f8cd161  #27315 minor optimizations to libsingular mpolys

comment:14 Changed 17 months ago by
 Status changed from needs_work to needs_review
comment:15 Changed 17 months ago by
The added import
+from sage.rings.finite_rings.finite_field_base cimport FiniteField
seems not to be used.
comment:16 followup: ↓ 18 Changed 17 months ago by
and about the change of "is_monomial", is this really the same ? I mean, does "monomial" mean coefficient 1 ?
comment:17 Changed 17 months ago by
 Commit changed from f8cd16194a7ee7d4adf482cb07b2b6c79d07f812 to 41fe6dd7965ee6867fc8c62dd1a52ee98f5de383
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
41fe6dd  #27315 minor optimizations to libsingular mpolys

comment:18 in reply to: ↑ 16 Changed 17 months ago by
comment:19 Changed 17 months ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok, then let it be. Merci
comment:20 Changed 17 months ago by
 Branch changed from u/mmezzarobba/mpoly_minor_opt to 41fe6dd7965ee6867fc8c62dd1a52ee98f5de383
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
#26067 Speed up exact division in ℤ[x,y,...] (again)
minor cleanup
mpoly //: minor optimization
#27315 minor optimizations to libsingular mpolys