training-operator icon indicating copy to clipboard operation
training-operator copied to clipboard
verified publisher icon kubeflow

Implement UTs for Plugin CustomValidations

Open tenzen-y opened this issue 9 months ago • 13 comments

What you would like to be added?

It would be great to add more fine-grained UTs for CustomValidations in each plugin.

  • [x] MPI: @tenzen-y
    • Target Function: https://github.com/kubeflow/trainer/blob/8aa97a4806b00752acac14fbad2cdee6e36d40c2/pkg/runtime/framework/plugins/mpi/mpi.go#L82
    • PR: https://github.com/kubeflow/trainer/pull/2555
  • [ ] JobSet: @Harshal292004
    • Target Function: https://github.com/kubeflow/trainer/blob/8aa97a4806b00752acac14fbad2cdee6e36d40c2/pkg/runtime/framework/plugins/jobset/jobset.go#L78
    • PR:
  • [x] Torch: @IRONICBo
    • Target Function: https://github.com/kubeflow/trainer/blob/8aa97a4806b00752acac14fbad2cdee6e36d40c2/pkg/runtime/framework/plugins/torch/torch.go#L55
    • PR: https://github.com/kubeflow/trainer/pull/2560

Why is this needed?

This allows us to strictly verify whether or not each validation works correctly.

Love this feature?

Give it a 👍 We prioritize the features with most 👍

tenzen-y avatar Mar 21 '25 12:03 tenzen-y

If anyone wants to take either plugin, please reach out to me. I can assign one of them to you.

tenzen-y avatar Mar 21 '25 12:03 tenzen-y

@tenzen-y May I take the MPI and JobSet Plugins

Harshal292004 avatar Mar 21 '25 12:03 Harshal292004

@tenzen-y May I take the MPI and JobSet Plugins

MPI already has been opened. So, I can assign JobSet to you.

tenzen-y avatar Mar 21 '25 12:03 tenzen-y

Please assign Torch to me, Thanks! @tenzen-y.

IRONICBo avatar Mar 21 '25 12:03 IRONICBo

Please assign Torch to me, Thanks! @tenzen-y.

Sure, I assigned Torch to you

tenzen-y avatar Mar 21 '25 12:03 tenzen-y

/assign @tenzen-y @Harshal292004 @IRONICBo

tenzen-y avatar Mar 21 '25 12:03 tenzen-y

@tenzen-y What exactly needs to be done here ?

Harshal292004 avatar Mar 22 '25 06:03 Harshal292004

Do I need to increase the test coverage for JobSet validation?

Harshal292004 avatar Mar 22 '25 06:03 Harshal292004

Do I need to increase the test coverage for JobSet validation?

I think you are right, we can refer to @tenzen-y PR , https://github.com/kubeflow/trainer/pull/2555.

IRONICBo avatar Mar 22 '25 11:03 IRONICBo

Hey @tenzen-y , Could you assign me jobset ?

Garvit-77 avatar Mar 22 '25 11:03 Garvit-77

Hey @Garvit-77 I am working on that

Harshal292004 avatar Mar 22 '25 12:03 Harshal292004

/reopen

tenzen-y avatar Mar 26 '25 08:03 tenzen-y

@tenzen-y: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

google-oss-prow[bot] avatar Mar 26 '25 08:03 google-oss-prow[bot]

@tenzen-y I have implemented JobSet validate tests . Could you please review it ?

Harshal292004 avatar Apr 03 '25 05:04 Harshal292004