Skip to content

Fix workload targets and max runtimes in docs#685

Merged
priyakasimbeg merged 1 commit intomlcommons:devfrom
runame:fix-docs
Mar 6, 2024
Merged

Fix workload targets and max runtimes in docs#685
priyakasimbeg merged 1 commit intomlcommons:devfrom
runame:fix-docs

Conversation

@runame
Copy link
Contributor

@runame runame commented Mar 5, 2024

Fixes #660 and #684.

@priyakasimbeg I noticed that we don't have a shared parent class for the librispeech deepspeech workloads and specify the targets, max runtime, and other properties twice -- once for Jax and once for PyTorch. Is there any reason for that? If not we should probably create a separate workload parent class. Otherwise it is easy to introduce a bug by accidentally making these values inconsistent.

@runame runame added the 🐛 Bug Something isn't working label Mar 5, 2024
@runame runame requested a review from priyakasimbeg March 5, 2024 22:39
@runame runame requested a review from a team as a code owner March 5, 2024 22:39
@github-actions
Copy link

github-actions bot commented Mar 5, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

This was linked to issues Mar 5, 2024
Mismatch between Maximum Runtime in the DOCS and max_allowed_runtime_sec in the code #660
Closed
Mismatch between target values in workload.py files and on DOCUMENTATION.md page #684
Closed
@priyakasimbeg
Copy link
Contributor

priyakasimbeg commented Mar 5, 2024 •
edited
Loading

Hi Runa,
Thanks a ton for working on this.

I noticed that we don't have a shared parent class for the Librispeech DeepSpeech workloads and specify the targets, max runtime, and other properties twice -- once for Jax and once for PyTorch. Is there any reason for that?

Right they inherit from the conformer JAX and PyTorch workloads to reuse the input pipeline and other methods. We actually had a bug until a few months ago where the DeepSpeech workloads were inheriting all the properties from the conformer workloads (including targets). The targets and runtimes were specified in a Librispeech DeepSpeech parent workload that was not being used so I removed it (#526).
At the time I took a stab at separating DeepSpeech from Conformer but it quickly turned into a mess with a large amount of duplicate code.
This is definitely a good thing to fix at some point in the future but I don't think we want to go down that rabbit hole right now.

@priyakasimbeg priyakasimbeg merged commit 68a0d18 into mlcommons:dev Mar 6, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@priyakasimbeg priyakasimbeg priyakasimbeg approved these changes

Assignees

No one assigned

Labels

🐛 Bug Something isn't working

Projects

None yet

Milestone

No milestone

2 participants

@runame @priyakasimbeg