Organized Simplifications and Edited Sgn Functions #221
Conversation
| @@ -163,7 +163,10 @@ export const ARITHMETIC_LIBRARY: IdentifierDefinitions[] = [ | |||
| type: ([x]) => x.type, | |||
| sgn: ([x]) => { | |||
| if (x.is(0)) return 'zero'; | |||
| return x.isNumberLiteral ? 'positive' : 'non-negative'; | |||
| if (x.isNumberLiteral) { | |||
There was a problem hiding this comment.
Isn't that redundant? The next line also checks isNumberLiteral
| @@ -183,13 +186,12 @@ export const ARITHMETIC_LIBRARY: IdentifierDefinitions[] = [ | |||
| type: addType, | |||
|
|
|||
| sgn: (ops) => { | |||
| if (ops.some((x) => x.isReal === false || x.isNaN)) return 'unsigned'; | |||
| if (ops.some((x) => x.isNaN)) return 'unsigned'; | |||
There was a problem hiding this comment.
Isn't that going to return undefined for complex numbers? Shouldn't it return 'unsigned'? I suppose that if two complex numbers could cancel each other out...
| @@ -273,12 +275,14 @@ export const ARITHMETIC_LIBRARY: IdentifierDefinitions[] = [ | |||
|
|
|||
| sgn: (ops) => { | |||
| const [n, d] = [ops[0], ops[1]]; | |||
| const s = d.sgn; | |||
| const s1 = n.sgn; | |||
There was a problem hiding this comment.
It's better to avoid calculating the sign of d and n until needed. In this case, they're not needed until after if (d.is(0))...
| if (d.isNegative) return oppositeSgn(n.sgn); | ||
| if (n.is(0) || (n.isFinite && d.isInfinity)) return 'zero'; | ||
| if (!n.is(0) && !d.is(0)) return 'not-zero'; | ||
| if (d.isPositive) return s1; |
There was a problem hiding this comment.
isPositive will have the side effect of calculating the sign of d. Since the sign is already in s, it's better to use positiveSign(s) instead.
| if (n.is(0) || (n.isFinite && d.isInfinity)) return 'zero'; | ||
| if (!n.is(0) && !d.is(0)) return 'not-zero'; | ||
| if (d.isPositive) return s1; | ||
| if (d.isNegative) return oppositeSgn(s1); |
| return s === 'positive' || s === 'not-zero' || s === 'negative' | ||
| ? 'positive' | ||
| : 'non-negative'; | ||
| } | ||
| if (a.type === 'imaginary') return 'negative'; |
There was a problem hiding this comment.
This checks that a is of type imaginary (and nothing else, i.e. that it is a pure, non-finite, imaginary number). Probably want to use subType(a.type, 'complex') or subType(a.type, 'imaginary') to exclude complex numbers with a real part.
| return undefined; | ||
| }, | ||
| canonical: (args, { engine }) => { | ||
| args = checkNumericArgs(engine, args, 2); | ||
| const [base, exp] = args; | ||
| return canonicalRoot(base, exp); | ||
| }, | ||
| evaluate: ([x, n], { numericApproximation }) => | ||
| root(x, n, { numericApproximation }), | ||
| evaluate: ([x, n], { numericApproximation }) => x.root(n), |
There was a problem hiding this comment.
This is incorrect. The correct result is root(x, n, { numericApproximation }).
It looks like you may be working off an out-of-date version of the repo and are merging older versions with your changes.
| ? 'positive' | ||
| : 'non-negative'; | ||
| } | ||
| if (x.type === 'imaginary') return 'negative'; |
There was a problem hiding this comment.
Same comment as above regarding imaginary. Probably want to use subType()
No description provided.