-
Notifications
You must be signed in to change notification settings - Fork 7k
[Data] Increase scale of map_groups release tests from SF10 to SF100
#58710
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
Conversation
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a fixed_size cluster variant to the map_groups release test. To make it pass, the scale factor (sf) was reduced from 100 to 10. My review points out that this change also reduces the test scale for the existing autoscaling variant, potentially lowering test coverage. I've suggested a way to use different scale factors for each cluster type. I also noted that a comment explaining why the test was limited to autoscaling clusters is now outdated and should be updated or removed.
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
map_groups release testmap_groups release tests from SF10 to SF100
ray-project#58710) > Thank you for contributing to Ray! 🚀 > Please review the [Ray Contribution Guide](https://docs.ray.io/en/master/ray-contribute/getting-involved.html) before opening a pull request. >⚠️ Remove these instructions before submitting your PR. > 💡 Tip: Mark as draft if you want early feedback, or ready for review when it's complete. ## Description > Briefly describe what this PR accomplishes and why it's needed. ray-project#58711 decreased the scale of the `map_groups` tests from scale-factor 100 to scale-factor 10 because some of the `map_groups` release tests were failing. However, after more investigation, I realized that the only variant that doesn't work with scale-factor 100 is the hash shuffle with autoscaling variant (see ray-project#58734). This PR re-increases the scale and only disables the cases that fail. ## Related issues > Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to ray-project#1234". ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Description
#58711 decreased the scale of the
map_groupstests from scale-factor 100 to scale-factor 10 because some of themap_groupsrelease tests were failing. However, after more investigation, I realized that the only variant that doesn't work with scale-factor 100 is the hash shuffle with autoscaling variant (see #58734).This PR re-increases the scale and only disables the cases that fail.