search

ebadawy / voice_conversion

MIT License
129 stars 36 forks source link

Fix outdated requirements.txt, which can lead to errors in training. #27

Closed flancast90 closed 1 year ago

flancast90 commented 1 year ago

This is a simple PR, which addresses #26, where I was getting an error during training. After debugging with @magicse, I was able to determine the cause of the issue to be a discrepancy between the requirements.txt package versions, and what @magicse had working on their device. After fiddling with dependency versions, and dependency-of-dependencies versioning, I was able to fix my issue with the proposed requirements.txt.

I have also added comments to the requirements.txt detailing why the specific versions are necessary, as, at face value, it seems that the latest of every package should work. I'm happy to answer any questions about this PR, but since it's a pretty minor "ease-of-use" change, I expect it should be an easy merge.

ebadawy commented 1 year ago

If the issue with numba, why changing the version for other packages? it is risky to change all packages' versions as their might be change in a function call that breaks the code.

flancast90 commented 1 year ago

If the issue with numba, why changing the version for other packages? it is risky to change all packages' versions as their might be change in a function call that breaks the code.

I went through everything, from processing, training, inference, etc, with no issues with these packages. The issue is with librosa, which relies on a specific version of numba, and numpy needs a different version of numba which then breaks numpy. Changing all of the versions to what's shown in the file ensures that others don't have this same issue. Also, these versions, besides me testing with them, are in use by @magicse, so that's 2 people that have no issues using these.