feat: unify jobs + mmt under jobs command - #68
Conversation
tejapulagam
changed the title
tejapulagam
changed the title
|
This should match with the SDK though, so we'd need to merge both classes as well (which would be doable). |
8cc205e to
55e8290
Compare
e03aa9a to
e4ac9b7
Compare
| "cloud_account_auth": cloud_account_auth, | ||
| "entrypoint": entrypoint, | ||
| "path_mappings": path_mappings_dict, | ||
| "num_machines": num_machines, |
There was a problem hiding this comment.
Side note: In order to move LitData to use MMT, we need to support per rank env variables, so each rank can adapt its behaviour
There was a problem hiding this comment.
happy to look into this as a next step when i get the time :)
| class MMT(Job): | ||
| """Compatibility interface for multi-machine jobs. | ||
|
|
||
| Multi-machine functionality is implemented by :class:`lightning_sdk.job.Job`. |
| self._cloud_account_api = CloudAccountApi() | ||
| self._job_api = JobApiV2() | ||
| self._standalone_job_api = JobApiV2() | ||
| self._mmt_job_api: Optional[MMTApiV2] = None |
There was a problem hiding this comment.
what's the issue with always instantiating it? Might make your typing a bit easier :)
There was a problem hiding this comment.
switched to always instantiate MMTApiV2()!
| if kind not in ("standalone", "multi"): | ||
| raise ValueError(f"Unknown job resource kind: {kind}") | ||
| self._resource_kind = kind | ||
| if kind == "multi": |
There was a problem hiding this comment.
what do we need kind for? Just the switch here? you could just make sure the job_api and mmt_api have the same public interface (they almost have i think).
And then just have a property that dynamically forwards based on num_machines or so?
This one works as well though :)
| job._job = j2 | ||
| jobs.append(job) | ||
| for m2 in self._teamspace_api.list_mmts(teamspace_id=self.id): | ||
| job = Job(name=m2.name, teamspace=self, _fetch_job=False, _resource_kind="multi") |
There was a problem hiding this comment.
is this _resource_kind required? Would it resolve this automatically? I don't have strong objections here, i just think it'd be better to not expose this at all if we can avoid this :)
There was a problem hiding this comment.
removed, now listing uses _attach_job(proto), which detects multi from V1MultiMachineJob and syncs num_machines
3920403 to
79f2fb7
Compare
4 participants
Unify standalone Jobs and multi-machine training (MMT) under a single
Jobresource, while keepingMMTandlightning mmtas a silent compatibility layer. I will setup a deprecation notice in a followup.Jobhandles both single- and multi-machine work (standalone/multibackends).Job.run(..., num_machines=N)creates a multi-machine job whenN > 1.is_multi_machine,num_machines, and rank-sortedmachines; stop/delete always target the parent.MMTis a thinJobsubclass for compatibility (no deprecation warnings yet).teamspace.jobslists both;teamspace.multi_machine_jobsis unchanged.lightning jobuses unifiedJob;lightning mmtstill uses theMMTwrapper.JobStepacceptsnum_machinesand creates an MMT step when> 1.