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

Practical productization enhancements #2

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cbruiz
Copy link

@cbruiz cbruiz commented Nov 3, 2023

Few changes proposed to facilitate integration in embedded projects.
Most important could be the ability to process gcode words with rust match templates almost easily.

  • Use lightweight fixed decimal representation to facilitate matching and avoid precision issues.
  • Internal (num, ord) decimal logic reinterpretation.
  • Make f64 optional so it could be non fully/completely supported in baremetal.

* Use lightweight fixed decimal representation to faciliate matching and avoid precision issues.
* Internal (num, ord) decimal logic reinterpretation.
* Make f64 optional so it could be non fully/completely supported in baremetal.
Copy link
Owner

@ithinuel ithinuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution.

Removing the requirement for f64 would be welcome indeed!
However, I am not sure that what is in this PR really makes a positive impact on the generated binary.

I had initially implemented it using binary scaling fixed point arithmetic.
It is a shame I got rid of the branches with the initial implementation 🤦

At the time, I ran tiny benchmark to compare the generated output binary.
Using the ATSAM4E (Cortex-M4 with single precision FPU), and keeping a position accuracy to 10^-5 mm which is the accuracy used by PrusaSlicer for the extruder axis. That is 17bits for the fractional part, 14bits for the integer part (on a signed integer) to fit a 32bits variable.
This limited the length to about 16meters. Although it may look fine for X,Y and Z, the slicer may emit an E position that's beyond that limit.

Implementing math for these (eg using https://crates.io/crates/fixed with its FixedI32<U17>) was increasing the binary size by about as much as what the double-precision float (which is also software).

So I figured that f64 was providing better precision (53bits mantissa ~ 15.95 decimal digits) at the expanse of some ram to store 64bits rather than 32.

The representation proposed in this PR effectively also takes 64bits (after accounting for padding & alignment) but does not implement the common math operations.

So here are a few questions that I'd like answered before proceeding:

  • Do you have figure to sustaining that this is effectively lighter weight (after considering math/conversion etc)?
  • This DecimalRepr seems to me like a manual version of floating point (32bit signed mantissa + 8bit unsigned? exponent). What is the benefit over a f64 and/or, say, a FixedI64<U17> fixed point representation?

src/types.rs Outdated Show resolved Hide resolved
src/parser/values.rs Outdated Show resolved Hide resolved
src/parser/values.rs Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@cbruiz cbruiz requested a review from ithinuel November 7, 2023 23:34
@cbruiz
Copy link
Author

cbruiz commented Nov 7, 2023

So here are a few questions that I'd like answered before proceeding:

  • Do you have figure to sustaining that this is effectively lighter weight (after considering math/conversion etc)?

My need was related to the comparison of gcodes in a higher level in order to facilitate matching by constant avoiding precision deviations.

  • This DecimalRepr seems to me like a manual version of floating point (32bit signed mantissa + 8bit unsigned? exponent). What is the benefit over a f64 and/or, say, a FixedI64<U17> fixed point representation?

Yes. It's manual. I didn't wanted to put a constraint by selecting one of the many crates providing fixed point or just use f64 by activating the feature. I think is more practical. The manual representation implemented is close to rust_decimal.

rust_decimal has HUGE precision and memory cost (more than I need), but also provides several math operations I needed... at least to evaluate. Nevertheless, as said.. maybe is good to let the consumer decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants