REQ sketch

[pmcgleenon]

Hello

thanks for creating this project!

There is a rust implementation of Relative Error Quantiles sketch derived from the C++ apache datasketches reference here https://github.com/pmcgleenon/reqsketch-rs

Maybe it could be used as a starting point for a REQ implementation in this project, it would need changes for the serialization and maybe other areas as well as a careful review

What do you think about adapting this existing rust implementation, or do you think a fresh start would be better?

[tisonkun]

@pmcgleenon either is OK for me.

To reduce review burden, I'd prefer a version as closed to the C++ impl as possible. The patch would better note any significant changes from the C++ or other referenced impl. Then we can use that as a starting point to make more changes.

You can take the commit history of CpcSketch as an example.

And yes, keep the serde impl compatible with other impl is significant since datasketches work hard to make all sketches interoperable among languages.

[Xuanwo]

Let me take a look.

[pmcgleenon]

@Xuanwo interested in your thoughts. I prototyped adding apache-datasketches compatible serialization to https://github.com/pmcgleenon/reqsketch-rs and it seems straightforward to add this

Do you have any opinions or guidance on the current reqsketch-rs API? If you want I can create a PR for review

By the way there is a benchmark that compares different types of sketches https://github.com/drmingdrmer/rust-histogram-benchmark, it's using version 0.0.1 of reqsketch-rs. There have been some improvements since 0.0.1 to align closer with the C++ apache datasketches implementation

[tisonkun]

@pmcgleenon Thanks for reaching out!

It seems reqsketch-rs is inspired by datasketches' impl. Is its interface similar to datasketches'? If so, would you like to upstream your reqsketch to this lib? I'm glad to review any pull request in this direction.

[tisonkun]

For example, @andylokandy and I previously made tdigests but now we prefer this lib's tdigest impl. Similar to @MrCroxx's cmsketch (foyer-rs/foyer#1294)

[tisonkun]

@pmcgleenon Sorry it's a few time ago so I just noticed that you're the issue author. Then the message is clear and simple:

Please send a PR and ping me as a reviewer :D

[pmcgleenon]

@tisonkun thanks, sure I'll create a PR for it and tag you so that you can take a look. The interface is similar to the datasketches

[tisonkun]

@pmcgleenon any update here? This issue comes to me today and I'd like to review any PR that initializes the REQSketch impl.

[pmcgleenon]

@tisonkun thanks for the reminder. I've got something working locally and integrated it with the datasketches serialization. It needs a bit more work before it's ready for review, I'll try to get some time on it this week to finish it up

[tisonkun]

First version delivered at #204.