Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cast IntegerNode to float32 in operations like f32 == 1.5 #609

Open
daisy1754 opened this issue Mar 20, 2024 · 11 comments
Open

Cast IntegerNode to float32 in operations like f32 == 1.5 #609

daisy1754 opened this issue Mar 20, 2024 · 11 comments
Labels

Comments

@daisy1754
Copy link

When you try to compare floating number with floating literal like Number == 12.34, it returns false even when Number is 12.34

Below is code to reproduce
https://go.dev/play/p/FFgPkXjIURm

package main

import (
	"fmt"

	"github.com/expr-lang/expr"
)

type Env struct {
	Number float32
}

func main() {
	code := `Number == 12.34`

	program, err := expr.Compile(code, expr.Env(Env{}))
	if err != nil {
		panic(err)
	}

	env := Env{
		Number: 12.34,
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}

	fmt.Println(output)
}
@daisy1754 daisy1754 changed the title Floating number comparison Floating number comparison returns incorrect result Mar 20, 2024
@antonmedv
Copy link
Member

This is how float comparison works:

package main

func main() {
	var f32 float32 = 12.34
	var f64 float64 = 12.34
	println(float64(f32) == f64) // false
}

https://go.dev/play/p/XsvrQ66pfrX

@daisy1754
Copy link
Author

@antonmedv I think example I gave is more like below. It is true in golang but false in expr

package main

func main() {
	var f32 float32 = 12.34
	println(f32 == 12.34) // true
}

https://go.dev/play/p/Z9SwZHGSi_J

@antonmedv
Copy link
Member

In golang f32 == 12.34, right hand side 12.34 will be interpreted as float32, i.e. float32(12.34). IN Expr 12.34 is a float64.

@daisy1754
Copy link
Author

well in golang literals are automatically casted to proper type right? in below example both float32 and float64 comparison returns true.

package main

func main() {
	var f32 float32 = 12.34
	var f64 float64 = 12.34
	println(f32 == 12.34) // true
	println(f64 == 12.34) // true
}

@antonmedv
Copy link
Member

Yes, type in inherited from left.

@daisy1754
Copy link
Author

and that is not the case for expr (please refer to my initial example) and that's why I opened issue. I believe Expr internally always cast number to flat64

@antonmedv
Copy link
Member

In your example number is float32

@daisy1754
Copy link
Author

Is this example more clear? I expect all to be true

package main

import (
	"fmt"

	"github.com/expr-lang/expr"
)

type Env struct {
	F32 float32
	F64 float64
}

var env = Env{
	F32: 12.34,
	F64: 12.34,
}

func main() {
	fmt.Printf("f32 expr: %t, go: %t\n", eval(`F32 == 12.34`), env.F32 == 12.34)
	fmt.Printf("f64 expr: %t, go: %t\n", eval(`F64 == 12.34`), env.F64 == 12.34)
}

func eval(code string) bool {
	program, err := expr.Compile(code, expr.Env(Env{}))
	if err != nil {
		panic(err)
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}
	return output.(bool)
}

result is

f32 expr: false, go: true
f64 expr: true, go: true

https://go.dev/play/p/GH9DSp3YFHF

@antonmedv
Copy link
Member

In first example expr does this

F32 == float64(12.34)

which is false.

@daisy1754
Copy link
Author

yeah I understand internal - that's why I sent out #610

Don't you think it's confusing though? As shown above

fmt.Printf("f32 expr: %t, go: %t\n", eval(F32 == 12.34), env.F32 == 12.34)

expr expression is false and golang expression is true in this case

@antonmedv
Copy link
Member

Yes! I think we can work on a proper solution for this. The solution in #610 is not correct.

What we need is to cast type to float32 only in case of lhs is float32. This should be done in patcher.

@antonmedv antonmedv reopened this May 25, 2024
@antonmedv antonmedv changed the title Floating number comparison returns incorrect result Cast IntegerNode to float32 in operations like f32 == 1.5 May 25, 2024
@antonmedv antonmedv added bug and removed feature labels Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants