Speaker diarization final#12
Conversation
…dane-speaker-diarisation-worker into speaker-diarization-final
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
Veldhoen
left a comment
Veldhoen
left a comment
There was a problem hiding this comment.
Hey Marta,
Thanks for your PR! I have left quite some remarks. Though I know you won't be able to work on this anymore, it would be great if you can make sure to leave this repo in a neat state. Also, I think it would be educational for you as a developer to go through the process of submitting a PR until having it approved. It's such a pity we haven't gone through this process yet!
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
| del msdd_model | ||
| torch.cuda.empty_cache() |
There was a problem hiding this comment.
This makes sense in a local environment, when deploying the model you probably do NOT want to remove the model from memory - so at least make this configurable
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
There was a problem hiding this comment.
Please specify the requirements in the pyproject.toml
(Or leave it out completetly: remove anything you have not worked on from the PR/branch)
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
| from omegaconf import OmegaConf | ||
|
|
||
|
|
||
| def config_setup(output_dir): |
There was a problem hiding this comment.
Consider supplying your own, augmented version of https://raw.githubusercontent.com/NVIDIA/NeMo/main/examples/speaker_tasks/diarization/conf/inference/diar_infer_{DOMAIN_TYPE}.yaml
So you can separate any config from the code.
There was a problem hiding this comment.
Including it as a file in the repository?
There are three different types of configuration files, depending on the domain of the audio file. So I should put all three files in the repo? And you change the configurable parameters from the file itself?
There was a problem hiding this comment.
Please remove anything you have not worked on from the PR/branch
2 participants
#10
The main additions that are to be reviewed are the files: helpers.py, diarize.py and torun.py. Moreover, the notebook which has the same contents as the helpers.py and diarize.py files.
The rest of the files are directly copied from the example worker and not modified.