https://github.com/flyteorg/flytekit/blob/c8433ea2d2ecf362815b1ee63554549ced9acd75/plugins/flytekit-kf-pytorch/flytekitplugins/kfpytorch/task.py#L67 @polite-ability-4005 do you know what the use case is to specify the image in the worker/master spec in the new pytorch task config? Doesn’t this mess with flyte’s way of specifying the image in the task decorator?

Not Byron but I’ve implemented something similar for

dask

in case folks want to have a different image in one of the components. Docs (and what takes precedence) here

Thanks! @polite-ability-4005 do you know whether the precedence is handled the same way for the new pytorch config?

@limited-raincoat-94253 ^

if you don’t specify image for the replica group, it will just use the image you set in the decorator

Thus if one also doesn’t set one there, it uses the one from

pyflyte run

I guess?

yes

Thanks for the quick reply guys! I need a bit more help 🙏

from flytekit import task, workflow, Resources
from flytekitplugins.kfpytorch import Elastic, PyTorch


@task(
    task_config=PyTorch(
        num_workers=1
    ),
    requests=Resources(cpu="3", mem="3Gi"),
)
def train():
    print("Trained")


@workflow
def wf():
    train()

When I run this workflow with

flytekit==1.6.2

installed locally it runs, propeller image is also version 1.6.2. When I install latest master of flytekit, the task stays queued with this error:

E0606 16:05:16.771720       1 workers.go:102] error syncing 'development/f0024b9843ea14ed3b8c': failed at Node[n0]. RuntimeExecutionError: failed during plugin execution, caused by: failed to execute handle for plugin [pytorch]: number of worker should be more then 0

I guess to use the new pytorch task config I’ll need a new propeller image, right? Do we already have an image built in CI somewhere?

it should be backward compatible for users who still have older flytekitplugins-kf-pytorch

No I’m running this code.

oh you set it to 1

But I’m registering from flytekit latest master installed against flytepropeller 1.6.2

I don’t think flytepropeller has a version of 1.6.2. I might made a mistake in the readme. Can we use the release version instead?

can you help me testing using the new config?

Yes 🙂 I’ll end the day soon but tomorrow I’ll build a propeller image from latest master and run it in our staging cluster.

sure, I will do some testing on my end as well

I think the 1.6.2 should already include the changes I made

If this is the case (didn’t check), then something is broken, right?

will do today

Thanks

What Yubo said. We need a new flytepropeller image. Yubo's change is only present in release 1.6.2b0 of flytekit. We're preparing 1.7.0, going should be out tomorrow, which should have Yubo's change.

I tested it locally and seems the issue is there, let me do a fix tomorrow when I wake up

thanks!

In my opinion, there is no use case that require different images, but definitely different resources. I think the image option is more to keep consistent with other plugins like mpijob

I don’t have super strong opinions and don’t want to be annoying so please excuse upfront for starting this discussion ahah Kubeflow unfortunately doesn’t have much (any?) documentation about when one would want to customize certain aspects of their Job types. For instance one can use more than 1 master replica but I don’t know what this would mean/when one does this. Since Flyte is an abstraction layer on top of K8s that hides away the details that ML engineers and data scientists don’t want to deal with, I think it would be nice to shield users from the options that even we as power users don’t know any use case for. (I can well imagine that not everything in kubeflow training operators is well thought through given how the rest of the project looks like.) And I’m wondering whether images are a candidate for this in PytorchJob, totally seeing that a power user would want to do customize the images for e.g. Spark, … • I’m curious what your use case if for different resources in master and worker (not because I’m arguing against it but because I actually don’t know what the difference between master and worker is and would love to know. Have never seen good docs about it) • Do you know if using more than 1 master actually makes any sense? • Can we as power users/someone who has used raw kubeflow pytorhjob before come up with a use case for different images in PytorchJob? If not, should we expose this to the user? What would we write in the docstring for when to do this?

so actually, I agree with your thoughts. so when I was working on this. Our use cases are 1. TFJob where ps/chief replicas require different resources than worker replicas 2. MPIJob where launcher replicas have different command than worker replicas. At that time my thoughts are, Flyte does not allow user to set specific details on the kubeflow jobs and I want to expose those fields for users. However, now I felt like I agree with your thoughts. Maybe we should hide users from the complexity. So as for now, I can either: 1. revert pytorch plugin to the original state or 2. Make basic usage more intuitive, so that users are not aware of other fields that can be set.

Are you joining the contributors’ sync today? 🙂 Maybe we can discuss there

sounds like a good idea. I will attend. I think we need a collective decision. I am not a pytorch expertise. My changes are based on my presumption on other kubeflows. Thanks for bringing this up though!

Thanks for coming to the meeting to discuss this @limited-raincoat-94253. I find the two arguments convincing that 1) with the other plugins we strive for parity with the underlying CRDs and 2) that it’s better to expose one parameter too much even though it’s not of much use to most people rather than omitting one that could have been useful to somebody.