gcd shows wrong result when input number is too large

[abiriadev]

Actual Behavior

>> gcd(500000000000000000, 500000000000000002)
500 000 000 000 000 000 ≈ 5×10^17

Expected result

2

Suggestion

I think it would be more helpful to warn a user if the input value is larger than some amount.
And I am willing to contribute or send PR if needed.

Regards.

[PaddiM8]

Hm yeah. Not even sure why this happens. It would make sense to warn the user, but I'm not sure there's a way to do that currently, while still giving a result. If we figure out when exactly this happens, it might make sense to simply return an error if the input is too large (for now at least). It seems to be when both are a certain size, but I haven't found a more specific pattern yet.

[abiriadev]

I'd be happy to help or contribute in any way if I can (and have enough time).

Could you please point me to where the gcd implementation is located? I think it might be a good starting point to identify the issue.

[PaddiM8]

Great! That might speed things up, since I also have limited time right now. The function is implemented here:

kalker/kalk/src/prelude/mod.rs

Line 612 in 9b55f89

pub fn gcd(x: KalkValue, y: KalkValue) -> Result<KalkValue, KalkError> {

KalkValue represents a value in kalk, usually a number. Normally, this is a heap allocated big float, but for the web version it's just an f64, since the big float library doesn't support WebAssembly. To be able to write generic code despite this, there is a float! macro that turns a number into the relevant type.

[dvishal485]

How can float! be useful here? I don't think it will resolve issue regarding number not being able to stored in heap with precision. Am I missing something here?

kalker/kalk/src/kalk_value/mod.rs

Lines 23 to 38 in 9b55f89

	#[macro_export]
	#[cfg(not(feature = "rug"))]
	macro_rules! float {
	($x:expr) => {{
	$x.clone() as f64
	}};
	}
	#[macro_export]
	#[cfg(feature = "rug")]
	macro_rules! float {
	($x:expr) => {{
	use rug::Float;
	Float::with_val(63, $x)
	}};
	}

I would love to contribute if I understand the issue completely.

[PaddiM8]

The purpose of float! is to create a rug::Float or f64, depending on if the rug feature is enabled. Unfortunately it doesn't get the configured precision.

[dvishal485]

I believe it has something to do with the fact that float! macro always get an f64 value because parsing of Literals was done with f64.

kalker/kalk/src/ast.rs

Line 22 in e9c9e81

Literal(f64),

kalker/kalk/src/ast.rs

Lines 13 to 30 in e9c9e81

	/// A tree structure of an expression.
	#[derive(Debug, Clone, PartialEq)]
	pub enum Expr {
	Binary(Box<Expr>, TokenKind, Box<Expr>),
	Unary(TokenKind, Box<Expr>),
	Unit(String, Box<Expr>),
	Var(Identifier),
	Group(Box<Expr>),
	FnCall(Identifier, Vec<Expr>),
	Literal(f64),
	Boolean(bool),
	Piecewise(Vec<ConditionalPiece>),
	Vector(Vec<Expr>),
	Matrix(Vec<Vec<Expr>>),
	Indexer(Box<Expr>, Vec<Expr>),
	Comprehension(Box<Expr>, Vec<Expr>, Vec<RangedVar>),
	Equation(Box<Expr>, Box<Expr>, Identifier),
	}

So no matter what you do with float!(), you will always be passing it f64 with less precision and hence value may be different than what it was asked to parse originally.

I don't know if I am right on this, but it is a best guess about the source of "error" I could make as a newbie.

[PaddiM8]

Hm yeah that sounds about right yeah. It should probably be a rug float when the rug feature is enabled.

[dvishal485]

I have changed a lot of things in codebase to fix this. Everything compiles and test out just fine with rug feature enabled as well as disabled. Making a PR for the same.

This would parse strings with correct precision instead of loading them into f64 and then making them Float. Tested with the input 500000000000000002 which works fine now. You may want to rearrange the codebase to accommodate some functions I added, and rename/remove some existing functions.

Unfortunately for without rug crate, it would use f64.