golemcloud / golem

Golem is an open source durable computing platform that makes it easy to build and deploy highly reliable distributed systems.
https://learn.golem.cloud/
Apache License 2.0
530 stars 59 forks source link

Use bigdecimal instead of f64 for Expr::number #969

Closed afsalthaj closed 1 month ago

afsalthaj commented 1 month ago

I think I didn't give much thoughts before raising this ticket. I am not seeing much value add here. If the current low-level wasm::rpc::Val based instruction set is fixed to support only primitive types like f32, f64, and integers, adding BigDecimal support in Expr (meaning parsing it as a bigdecimal before converting to primitive analysed type) won't provide significant advantages, since we would lose the precision of BigDecimal during conversion anyway. In other words, in my view, unless we are planning to extend the instruction set to handle high-precision decimal types, it's probably not worth the added complexity to parse and handle BigDecimal at the Expr level. Every calculations (arithmetic) is going to happen as an interpretation of byte code and not before!

I am untagging this ticket from 1.1 Rib, given the value add is lower, and we have so many other tickets in pipeline. I am replacing this with a new ticket to 1.1 epic that expands on test case in comprehensive test module in Rib interpreter.

Also please correct me if I am wrong here, or missed out on the basics.

afsalthaj commented 1 month ago

For the above reason, we can close this ticket