Skip to content
This repository has been archived by the owner on Oct 13, 2022. It is now read-only.

Sync snowfall with Lhotse's dataset/sampler refactoring PR #95

Merged
merged 2 commits into from
Feb 10, 2021

Conversation

pzelasko
Copy link
Collaborator

@pzelasko pzelasko commented Feb 9, 2021

Reflects the changes from lhotse-speech/lhotse#194

This new version allows working with distributed training out-of-the-box. Once it is merged, I will rebase #71 on that and finish multi-GPU training support.

One thing I am concerned about is the number of files with duplicate code. It becomes quite labor-intensive to propagate these kinds of changes, and I am not sure if I'm doing everything OK as I can't run all of these (🙏🏻 static checker tool helps 🙏🏻). It could be good to slowly converge to some common code structure that can be shared and imported (e.g. a Trainer class or something like that)?

@danpovey
Copy link
Contributor

How do you run the static checker tool?
I'm OK with leaving snowfall fairly unstructured. As far as I am concerned it is just a repo where we can iterate rapidly on experiments, and not a serious thing that we are "releasing"; we can do a cleaned-up repo later once we have an idea what kinds of models we'll be using.

@pzelasko pzelasko mentioned this pull request Feb 10, 2021
@pzelasko
Copy link
Collaborator Author

I use the one that is integrated into PyCharm and IMO it is the best tool available. I believe the next best thing is mypy, and it has integrations with multiple editors/IDEs. Years ago when these things were not available I used flake8 it says it's "style enforcement" but it also has some static checks (and a rich plugin ecosystem I think). In case you are using VSCode, perhaps pylance is interesting (it's exclusive to VSCode though).

@pzelasko
Copy link
Collaborator Author

OK this works - merging.

@pzelasko pzelasko merged commit c6c7b6d into k2-fsa:master Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants