Project

General

Profile

Design #1735

PushFront/PushBack without checks?

Added by John Abbott about 1 year ago. Updated about 2 months ago.

Status:
Rejected
Priority:
Normal
Assignee:
John Abbott
Category:
Improving
Target version:
CoCoALib-0.99850
Start date:
25 Apr 2023
Due date:
% Done:

100%

Estimated time:
1.11 h
Spent time:
1.05 h

Description

Currently PushFront and PushBack check their args (correct coeffring, correct ppmonoid).
Should there also be unsafe non-checking versions? I think it may make the fn faster.

2024 Conclusion: if you don't want the checks, call the member function. This was a guideline we had many years ago (and the forgot about?).

History

#1 Updated by John Abbott about 1 year ago

The suggestion arose from some new code for computing cyclotomic polynomials.
The coeff vec is computed quite quickly, but the conversion (from vector<long>) to a CoCoALib polynomial takes quite a lot longer than computing the coeffs (almost 10 times as long!)

I should check that disabling the arg checks actually makes it faster....

#2 Updated by John Abbott 2 months ago

As I stated in the comment just above: I should test whether a non-checking impl would really make it faster. Maybe the slow down is due to something else (mem mgt)?

#3 Updated by Anna Maria Bigatti 2 months ago

  • % Done changed from 0 to 10

Surely worth a try.
PushFront_nocheck?
I could imagine cases when this might be used (homomorphisms preserving ordering)

#4 Updated by John Abbott 2 months ago

  • Status changed from New to In Progress
  • Assignee set to John Abbott

I have just tried disabling the checks inside PushFront and PushBack. There was no difference when calling cyclotomic because that function calls the non-checking member function myPushFront directly.

The function reverse defined in SparsePolyOps-RingElem.C around line 1290 does call PushFront -- and it seems not many other functions do call PushFront. I created a large cyclotomic, and then reversed it. The timings are:

  • p = NextPrime(10000000)
  • Create cyclotomic(p,x) in ZZ[x] takes about 13.7s
  • reverse with checks takes about 39s
  • reverse without checks takes about 27s
  • reverse hacked to call myPushFront takes about 22.5s (this includes the time to divide PPs)

Conclusion: if you don't want the checks, call the member function. This was a guideline we had many years ago (and the forgot about?).

So reject?

PS PushBack does less checking than PushFront since it is not so easy to access that last PP (maybe?)

#5 Updated by John Abbott about 2 months ago

  • Status changed from In Progress to Rejected
  • % Done changed from 10 to 100
  • Estimated time set to 1.11 h

Even the documentation mentioned about myPushFront etc. I have slightly improved the doc.
Otherwise we can reject this since there was already a way achieve what was desired.

PS I did also improve the impl of reverse

#6 Updated by Anna Maria Bigatti about 2 months ago

  • Description updated (diff)

Also available in: Atom PDF