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

implement Serializable for ThresholdedRandomCutForestState #298

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

ylwu-amzn
Copy link
Contributor

Signed-off-by: Yaliang Wu ylwu@amazon.com

#297

Description of changes:
implement Serializable for ThresholdedRandomCutForestState

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Signed-off-by: Yaliang Wu <ylwu@amazon.com>
@jotok
Copy link
Contributor

jotok commented Feb 14, 2022

Per the recommendation in the Javadocs for Serializable, we should explicitly define the serialVersionUID constant to the classes that implement Serializable.
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/Serializable.html

@jotok
Copy link
Contributor

jotok commented Feb 14, 2022

Can we create an example based on the existing serialization examples that serializes and deserializes a forest and validates that the deserialized forest produces outputs that are consistent with the original forest?
https://github.com/aws/random-cut-forest-by-aws/blob/main/Java/examples/src/main/java/com/amazon/randomcutforest/examples/serialization/ProtostuffExample.java

@ylwu-amzn
Copy link
Contributor Author

Per the recommendation in the Javadocs for Serializable, we should explicitly define the serialVersionUID constant to the classes that implement Serializable. https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/Serializable.html

Thanks, will add. BTW, will evaluate other serialization lib later and may change the serialization way in MLCommons. Currently use ObjectOutputStream is mainly to keep consistent with current code and save effort to unblock some work.

@ylwu-amzn
Copy link
Contributor Author

existing serialization examples that serializes and deserializes a forest and validates that the deserialized forest produces outputs that are consistent wi

Sure, will add

Signed-off-by: Yaliang Wu <ylwu@amazon.com>
@ylwu-amzn
Copy link
Contributor Author

Can anyone help take a look and approve if no more comments? @jotok @sudiptoguha

@ylwu-amzn
Copy link
Contributor Author

ylwu-amzn commented Mar 7, 2022

Ran ObjectStreamExample on Mac successfully.

dimensions = 10, numberOfTrees = 50, sampleSize = 256, precision = FLOAT_32
Object output stream size = 326809 bytes
Looks good!

@jotok jotok merged commit 4b1d376 into aws:main Mar 7, 2022
ylwu-amzn added a commit to ylwu-amzn/random-cut-forest-by-aws that referenced this pull request Mar 25, 2022
* implement Serializable for ThresholdedRandomCutForestState

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* add serialVersionUID and example

Signed-off-by: Yaliang Wu <ylwu@amazon.com>
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