Skip to content

Conversation

@weibozhao
Copy link
Contributor

@weibozhao weibozhao commented Dec 9, 2022

What is the purpose of the change

  • Add HasSeed param for RandomSplitter.

Brief change log

  • Add API setSeed() for RandomSplitter Transformer in Java and Python.
  • This parameter guarantees reproduciable output only when the paralleism is unchanged and each worker reads the same data in the same order.
  • Add ut for this new API.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (yes)

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (docs)

@yunfengzhou-hub
Copy link
Contributor

Hi @weibozhao, The failure thrown in the CI of this PR has been fixed by a recently merged PR. Could you please rebase this PR onto the latest master code to ensure the tests can pass?

Copy link
Contributor

@yunfengzhou-hub yunfengzhou-hub left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Left some comments as below.

@yunfengzhou-hub
Copy link
Contributor

Hi @weibozhao, thanks for the update. Could you please modify the description section of this PR and its corresponding Jira ticket, explaining the changes made in this PR and why we should add them to Flink ML?

@weibozhao
Copy link
Contributor Author

Hi @weibozhao, thanks for the update. Could you please modify the description section of this PR and its corresponding Jira ticket, explaining the changes made in this PR and why we should add them to Flink ML?

I have modified the description of this PR.

@zhipeng93
Copy link
Contributor

@weibozhao Thanks for the update :) Can you also update the PR title as well as the commit message to make it more informative? (e.g., Add HasSeed param for RandomSplitter)

@weibozhao weibozhao changed the title [FLINK-30348] Refine Transformer for RandomSplitter [FLINK-30348] Add HasSeed param for RandomSplitter Jan 5, 2023
@weibozhao
Copy link
Contributor Author

OK

@weibozhao
Copy link
Contributor Author

@weibozhao Thanks for the update :) Can you also update the PR title as well as the commit message to make it more informative? (e.g., Add HasSeed param for RandomSplitter)

done.

@zhipeng93 zhipeng93 merged commit 6f8aa57 into apache:master Jan 6, 2023
vacaly pushed a commit to vacaly/flink-ml that referenced this pull request Feb 8, 2023
vacaly pushed a commit to vacaly/flink-ml that referenced this pull request Feb 10, 2023
Fanoid pushed a commit to Fanoid/flink-ml that referenced this pull request Apr 6, 2023
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.

3 participants