rtoy / maxima

A Clone of Maxima's repo
Other
0 stars 0 forks source link

expop/expon must be integers #2717

Closed rtoy closed 3 months ago

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:07 Created by macrakis on 2024-04-29 17:45:22 Original: https://sourceforge.net/p/maxima/bugs/4296


expop/expon should get the same assign property as maxposex/maxnegex:

simp.lisp

;; Check assignment to be a positive integer
(putprop '$expop 'posintegerset 'assign)
(putprop '$expon 'posintegerset 'assign)

Also, we might want to redefine posintegerset's message to be more accurate:

(defun posintegerset (x y)
  (if (or (not (integerp y))
          (not (>= y 0)))
      (merror
        (intl:gettext "assignment: '~:M must be a positive integer **or zero**. Found: ~:M")
        x y)))
rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:09 Created by rtoy on 2024-04-29 20:05:09 Original: https://sourceforge.net/p/maxima/bugs/4296/#e72a


Agreed.

In the original definition of $expop, it was declared to be a fixnum. We don't currently recognize such things mostly because there are bits of code that does things like:

(prog ($expop) ...
  (setf $expop 0)
  ..)

These should be fixed. But then it doesn't match what the assign property says. Should $expop really be a fixnum and should it be enforced? Maybe the declaration of $expop to be a fixnum doesn't really improve performance in any way. On the other hand, I don't see people setting $expop to anything near most-positive-fixnum. Maybe a few hundred or thousand at most?

Maybe that's not our problem and any positive integer is acceptable?

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:13 Created by macrakis on 2024-04-29 20:53:08 Original: https://sourceforge.net/p/maxima/bugs/4296/#e72a/d289


Bizarrely, expop can currently be a float:

(x+1)^2,expop:2.01 => expands
(x+1)^2,expop:2b0 => Lisp error
(x+1)^2,expop:5/2 => Lisp error

It should be restricted to a non-negative int aka fixnum.

Of course the (prog ($expop) ...) case needs to be fixed.

The max value is the user's problem, not ours. After all, (x+1)^5000,expop:10000 only takes 12 secs. Is it useful? Not our problem.

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:16 Created by rtoy on 2024-04-29 22:10:06 Original: https://sourceforge.net/p/maxima/bugs/4296/#e72a/d289/3c80


Of course it accepts anything because we don't check assignments. Totally agree that it should be a non-negative integer. posintegerset is badly named. We should rename it to non-negative-integer-set or something that actually reflects what the check is for.

Maybe I'll go and fix defmvar to recognize the fixnum declaration. Or change such things to be integer instead of fixnum. Or be more precise. Mostly to find places where we didn't initialize $expop correctly.

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:20 Created by rtoy on 2024-04-30 01:59:29 Original: https://sourceforge.net/p/maxima/bugs/4296/#40c4


You must be looking at an old version of the sources. Commit [f504c338e8] changed the message to say "non-negative integer" instead of just "positive integer". I think that's good enough.

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:23 Created by macrakis on 2024-04-30 14:11:43 Original: https://sourceforge.net/p/maxima/bugs/4296/#40c4/a084


agreed -- I just keep the sources of the pre-built version that I'm running

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:27 Created by rtoy on 2024-05-04 16:37:32 Original: https://sourceforge.net/p/maxima/bugs/4296/#34c1


I took a closer look at this stuff. Given how expop is used, there's no real advantage to declare expop to be a non-negative integer.   In the places where it is used, there's tons of code around it so any possible compiler optimization would be minimal.

I think the best we can do is to add an 'assign property so that the user can only set the variable to the appropriate allowed values.

I hacked defmvar to print a message whenever we use fixnum, boolean, string, or flonum (which are currently recognized but do nothing). The list is

FIXNUM for $FPPRINTPREC
BOOLEAN for $LISTCONSTVARS
FIXNUM for $FACTOR_MAX_DEGREE
FIXNUM for $EXPOP
FIXNUM for $MAXPOSEX
FIXNUM for $MAXNEGEX
FIXNUM for $LINENUM
BOOLEAN for $SAVE_PRIMES
FIXNUM for $PRIMEP_NUMBER_OF_TESTS
FIXNUM for $POLLARD_RHO_LIMIT
FIXNUM for $POLLARD_PM1_LIMIT
FIXNUM for $POLLARD_RHO_TESTS
FIXNUM for $POLLARD_PM1_TESTS
FIXNUM for $POLLARD_RHO_LIMIT_STEP
FIXNUM for $POLLARD_PM1_LIMIT_STEP
FIXNUM for $ECM_NUMBER_OF_CURVES
FIXNUM for $ECM_LIMIT
FIXNUM for $ECM_MAX_LIMIT
FIXNUM for $ECM_LIMIT_DELTA
BOOLEAN for $IFACTOR_VERBOSE
BOOLEAN for $FACTORS_ONLY
BOOLEAN for $FACTOR_MAX_DEGREE_PRINT_WARNING
FIXNUM for $TRACE_MAX_INDENT
BOOLEAN for $FORTSPACES
FIXNUM for $FORTINDENT
FIXNUM for $ZN_PRIMROOT_LIMIT
BOOLEAN for $ZN_PRIMROOT_VERBOSE
BOOLEAN for $ZN_PRIMROOT_PRETEST
BOOLEAN for $GF_RAT
BOOLEAN for $GF_SYMMETRIC
BOOLEAN for $GF_BALANCED
FIXNUM for $GF_COEFF_LIMIT
BOOLEAN for $GF_CANTOR_ZASSENHAUS

I didn't check them all, but the fixnum items are probably positive (or non-negative) integers). We should probably add an assign property for these. But that requires examining each one to see what makes sense.

For the boolean variables, it makes sense to allow the user to only set the variable to true or false, assuming the boolean option is used correctly.)

There are no uses of string or flonum.

So what I think I'm going to do here is just modify $expop and friends to allow only non-negative integers. No type declarations. And I'll modify boolean to generate code to allow just true and false values.

The other fixnum variables will get fixed some other day.

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:30 Created by macrakis on 2024-05-05 07:59:42 Original: https://sourceforge.net/p/maxima/bugs/4296/#34c1/3ab6


Yes, the assign property is what matters. Declaration as fixnum is a nanooptimization.

rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:34 Created by rtoy on 2024-05-17 19:16:54 Original: https://sourceforge.net/p/maxima/bugs/4296/#1cb0


rtoy commented 3 months ago

Imported from SourceForge on 2024-07-06 18:15:37 Created by rtoy on 2024-05-17 19:16:54 Original: https://sourceforge.net/p/maxima/bugs/4296/#7701


Fixed by commit [9fd63b2fb4deafd3d1bd04249f8414ac2aebfe08]. There are still a few variables with fixnum declarations that I didn't fix because I'm not that familiar with them. I left the deprecation messages in so that someday someone who knows better than me will fix them.