added ensemble model to docs #22771
added ensemble model to docs #22771
Conversation
|
JFYI: I'm going OOO, ping @simon-mo to merge when it's ready |
|
In addition there're some docs linter formatting error on this commit .. Here's a readme about how to locally run linter https://sourcegraph.com/github.com/ray-project/ray/-/blob/doc/README.md There's the error https://buildkite.com/ray-project/ray-builders-pr/builds/25782#00c390e0-8af9-49f4-b8c4-e13d09482406 with edit suggestions on the error message. Not sure if you can access it directly, it's https://gist.github.com/jiaodong/6839b1deb6259a9ba882711b1a98ba31. All other content looks great to me, thanks for contributing ! |
edoakes
added
the
@external-author-action-required
| print(all_predictions) | ||
|
|
||
|
|
||
| # start local cluster and ray serve processes |
There was a problem hiding this comment.
@jiaodong I'd argue that this snippet is a bit too long to simply put it at once into a docs section.
Do you think it would make sense to split this up a little and include this in parts? This way those good comments in the snippet can be actual text in the docs.
There was a problem hiding this comment.
I agree this helps with readability. Given this PR is contributed by community and Jan already made a number of commits following existing pattern I think it might make more sense I just clean them up together as a separate PR to reduce friction on our community end ? The snippet_model_ensemble is structured the same way as snippet_model_composition, we need to better format both.
There was a problem hiding this comment.
yes, indeed it is actually a little long. Maybe you can manage to add it as an example notebook or something similar. I think for users it is quite important to get the complete example as well.
|
@frosk1 can you rebase on master to resolve the merge conflict ? In addition the latest linter error is https://gist.github.com/jiaodong/b6a8b7b04facaf1ea68259b58253d78f that it starts to think differently about formatting after we shorten the |
frosk1
requested review from
sven1977,
gjoliver,
avnishn,
ericl,
fishbone,
scv119,
clarkzinzow,
jjyao,
jovany-wang,
kfstorm,
raulchen,
wuisawesome and
DmitriGekhtman
as code owners
|
@frosk1 maybe you want to try |
|
@jiaodong I was thinking that simply rebasing was not a good plan, since it would include all other commits. I could have used merge --squash. But now I have reverted the rebase and merged it into current master. Please accept now or merge it yourself later, since I am not aware of the release cycle or merge times for master :) |
|
thanks @frosk1 ! We have conflicting new tests in the BUILD file but the content and test lgtm. We can take it from here and I will push commits to this PR to deal with conflicts. |
|
@jiaodong do you plan on merging these in the near future? :) |
|
@frosk1 yes we are :) sorry in last two weeks we're in full sprinting mode to push PRs for upcoming branch cut so I didn't follow up on this yet, but next week should be ok again |
jiaodong
removed request for
ericl,
scv119,
jjyao,
raulchen,
kfstorm,
wuisawesome,
sven1977,
gjoliver,
clarkzinzow,
jovany-wang,
avnishn,
DmitriGekhtman and
fishbone
jiaodong
added
@author-action-required
| if __name__ == "__main__": | ||
| # start local cluster and ray serve processes | ||
| # Start ray with 8 processes. | ||
| if ray.is_initialized: |
There was a problem hiding this comment.
Just saw this -- it should be ray.is_initialized() I think :)
@edoakes
Why are these changes needed?
Added ensemble model examples to the Documentation. That was needed, due to a user request and there was no methodology outlining the creation of higher level ensemble models.
Related issue number
Checks
scripts/format.shto lint the changes in this PR.