[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs
From: |
Arun Giridhar |
Subject: |
[Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs |
Date: |
Wed, 1 Dec 2021 16:21:11 -0500 (EST) |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:94.0) Gecko/20100101 Firefox/94.0 |
Follow-up Comment #7, bug #61565 (project octave):
No, the colon operator has a lower precedence than the arithmetic ones, so the
parentheses don't make a difference.
It turned out the first nchoosek-specific bug was in this line, where k was
being rounded differently when it was an integer (rounded up for odd integers)
compared to a float (always rounded down):
denom = [(1:k/2) .* (k-1:-1:(k+1)/2) / 2, k];
That line needed to be changed to:
denom = [(1:(k-1)/2) .* (k-1:-1:(k+1)/2) / 2, k];
The reason this bug was not exposed until now was that the previous line in
nchoosek would trigger an error about ranges, so we didn't get to see this
behavior until the range bugs were fixed first. With the change above,
nchoosek (uint8(10), uint8(5)) evaluates correctly, and it is no longer a
regression from Octave 6.
But that change is not enough, because nchoosek still complains about
mismatched types when given different integer types:
>> nchoosek (uint8(11), int8(5))
error: binary operator '-' not implemented for 'uint8 scalar' by 'int8 scalar'
operations
error: called from
nchoosek at line 117 column 7
This problem is internal to the integer type system and not because of the
definition of the binomial coefficient. The simplest solution is to cast both
v and k to doubles. Losing precision for v and k above flintmax is not a
problem because nchoosek is required (for Matlab compatibility) to return a
double and any result that exceeds flintmax will trigger a warning anyway.
(Casting v and k to double does not introduce new loss of precision.)
So the two changes to be made are to convert k/2 to (k-1)/2 in the line above,
and to convert v and k to double. Casting to double makes the first change
optional.
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?61565>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/01
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/01
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs,
Arun Giridhar <=
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/01
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Arun Giridhar, 2021/12/01
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Nicholas Jankowski, 2021/12/01
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Arun Giridhar, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Arun Giridhar, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Arun Giridhar, 2021/12/02
- [Octave-bug-tracker] [bug #61565] nchoosek broken for integer inputs, Markus Mützel, 2021/12/02