You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "Fengzdadi (via GitHub)" <gi...@apache.org> on 2026/01/23 05:57:18 UTC
[PR] feat(sampling): add varopt serde [datasketches-go]
Fengzdadi opened a new pull request, #108:
URL: https://github.com/apache/datasketches-go/pull/108
## Summary
- add VarOptItemsSketch implementation (variance-optimal weighted sampling)
- add VarOpt serialization/deserialization support
- add VarOpt unit tests
## Changes
- core sketch implementation and update logic
- serde preamble/flags encoding and decoding
- tests covering warmup/estimation, invalid weights, and cumulative weight property
ref: #98 PR2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on code in PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#discussion_r2723790034
##########
sampling/varopt_items_sketch.go:
##########
@@ -524,3 +529,256 @@ func (s *VarOptItemsSketch[T]) adjustedSize(maxSize, resizeTarget int) int {
}
return maxSize
}
+
+// VarOptItemsSketchEncoder writes a Java-compatible VarOpt sketch to an io.Writer.
+type VarOptItemsSketchEncoder[T any] struct {
+ w io.Writer
+ serde ItemsSerDe[T]
+}
+
+// NewVarOptItemsSketchEncoder creates an encoder with the provided writer and serde.
+func NewVarOptItemsSketchEncoder[T any](w io.Writer, serde ItemsSerDe[T]) *VarOptItemsSketchEncoder[T] {
Review Comment:
Can you return value not pointer? returning pointer can be allocated in heap. Encoder is used once to encoding. I don't think long life cycle. So avoiding allocated in heap is good.
```
sampling/varopt_items_sketch.go:541:9: &VarOptItemsSketchEncoder[go.shape.string]{...} escapes to heap in NewVarOptItemsSketchEncoder[go.shape.string]:
```
##########
sampling/varopt_items_sketch.go:
##########
@@ -524,3 +529,256 @@ func (s *VarOptItemsSketch[T]) adjustedSize(maxSize, resizeTarget int) int {
}
return maxSize
}
+
+// VarOptItemsSketchEncoder writes a Java-compatible VarOpt sketch to an io.Writer.
+type VarOptItemsSketchEncoder[T any] struct {
+ w io.Writer
+ serde ItemsSerDe[T]
+}
+
+// NewVarOptItemsSketchEncoder creates an encoder with the provided writer and serde.
+func NewVarOptItemsSketchEncoder[T any](w io.Writer, serde ItemsSerDe[T]) *VarOptItemsSketchEncoder[T] {
+ return &VarOptItemsSketchEncoder[T]{w: w, serde: serde}
+}
+
+// Encode writes the serialized sketch to the encoder's writer.
+func (e *VarOptItemsSketchEncoder[T]) Encode(sketch *VarOptItemsSketch[T]) error {
+ if e == nil || e.w == nil {
+ return errors.New("nil writer")
+ }
+ data, err := encodeVarOptItemsSketch(sketch, e.serde)
+ if err != nil {
+ return err
+ }
+ _, err = e.w.Write(data)
+ return err
+}
+
+// VarOptItemsSketchDecoder reads a Java-compatible VarOpt sketch from an io.Reader.
+type VarOptItemsSketchDecoder[T any] struct {
+ r io.Reader
+ serde ItemsSerDe[T]
+}
+
+// NewVarOptItemsSketchDecoder creates a decoder with the provided reader and serde.
+func NewVarOptItemsSketchDecoder[T any](r io.Reader, serde ItemsSerDe[T]) *VarOptItemsSketchDecoder[T] {
Review Comment:
Also can you return value?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi closed pull request #108: feat(sampling): add varopt serde
URL: https://github.com/apache/datasketches-go/pull/108
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi commented on code in PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#discussion_r2724646087
##########
sampling/varopt_items_sketch.go:
##########
@@ -524,3 +529,256 @@ func (s *VarOptItemsSketch[T]) adjustedSize(maxSize, resizeTarget int) int {
}
return maxSize
}
+
+// VarOptItemsSketchEncoder writes a Java-compatible VarOpt sketch to an io.Writer.
+type VarOptItemsSketchEncoder[T any] struct {
+ w io.Writer
+ serde ItemsSerDe[T]
+}
+
+// NewVarOptItemsSketchEncoder creates an encoder with the provided writer and serde.
+func NewVarOptItemsSketchEncoder[T any](w io.Writer, serde ItemsSerDe[T]) *VarOptItemsSketchEncoder[T] {
Review Comment:
got it, thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi commented on PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#issuecomment-3813278608
Thanks for pointing this out. I’ll go through the original implementation again and update the marks handling step by step to match it more closely, then push an update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi commented on PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#issuecomment-3866428647
This PR now has substantial conflicts/overlap with newer changes, and continuing here would make the history and discussion hard to follow.
I plan to close this PR and rework it on top of the latest main, then open a fresh PR and link back to this one. Is this OK？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on code in PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#discussion_r2723790034
##########
sampling/varopt_items_sketch.go:
##########
@@ -524,3 +529,256 @@ func (s *VarOptItemsSketch[T]) adjustedSize(maxSize, resizeTarget int) int {
}
return maxSize
}
+
+// VarOptItemsSketchEncoder writes a Java-compatible VarOpt sketch to an io.Writer.
+type VarOptItemsSketchEncoder[T any] struct {
+ w io.Writer
+ serde ItemsSerDe[T]
+}
+
+// NewVarOptItemsSketchEncoder creates an encoder with the provided writer and serde.
+func NewVarOptItemsSketchEncoder[T any](w io.Writer, serde ItemsSerDe[T]) *VarOptItemsSketchEncoder[T] {
Review Comment:
Can you return value not pointer? returning pointer can be allocated in heap. Encoder is used once to encoding. I don't think it has long life cycle. So avoiding allocated in heap is good.
```
sampling/varopt_items_sketch.go:541:9: &VarOptItemsSketchEncoder[go.shape.string]{...} escapes to heap in NewVarOptItemsSketchEncoder[go.shape.string]:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] feat(sampling): add varopt serde [datasketches-go]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on PR #108:
URL: https://github.com/apache/datasketches-go/pull/108#issuecomment-3866865656
Sounds good to me.
Thanks a lot for the continued work!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org