search

ray-project / ray_lightning

Pytorch Lightning Distributed Accelerators using Ray
Apache License 2.0
211 stars 34 forks source link

LightningCLI and RayPlugin compatibility #164

Closed mauvilsa closed 1 year ago

mauvilsa commented 2 years ago

Proper fix for #151 removing what #154 added.

cc @amogkam

amogkam commented 2 years ago

Thanks for the contribution @mauvilsa!

Could you add this test below this line https://github.com/ray-project/ray_lightning/blob/main/.github/workflows/test.yaml#L133 so that it gets run in CI?

Also seems like there are some lint failures. Could you run format.sh to fix those?

mauvilsa commented 2 years ago

Thanks for the contribution @mauvilsa!

Could you add this test below this line https://github.com/ray-project/ray_lightning/blob/main/.github/workflows/test.yaml#L133 so that it gets run in CI?

Also seems like there are some lint failures. Could you run format.sh to fix those?

@amogkam done.

mauvilsa commented 1 year ago

@amogkam I addressed your comment long ago. So long that the removal of the workaround was already done in https://github.com/ray-project/ray_lightning/commit/299a776e4348050c477cbb4ecf958fb03ed64f26. However, adding the unit test still make sense.

amogkam commented 1 year ago

Thanks @mauvilsa and apologies for the delay! Just re-triggered the CI again.

mauvilsa commented 1 year ago

I committed fix the unit test for the rename from RayPlugin to RayStrategy. @amogkam please re-trigger the CI.