claritychallenge / clarity

Clarity Challenge toolkit - software for building Clarity Challenge systems
https://claritychallenge.github.io/clarity
MIT License
130 stars 54 forks source link

219 support for python 311 #276

Closed jonbarker68 closed 1 year ago

jonbarker68 commented 1 year ago

Support for Python 2.11

Python 2.11 has required some key packages to need major version upgrades.

The upgrade to torchaudio has required a few further fixes

Other changes

All tests are now passing and this could be merged into main. It will however force any users to upgrade their torch and torchaudio packages.

jonbarker68 commented 1 year ago

The following torch-based regression tests are failing.

tests/predictor/test_torch_stoi.py ....FFFF.......
tests/regression/test_engine_losses.py ..FF
tests/regression/test_mc_conv_tasnet.py .F
tests/regression/test_predictors.py Fs.

I believe this may be because the random numbers have changed (this hypothesis needs checking...). In which case either the seeding needs fixing or it may be that even with the correct seeding there is no way to maintain reproducibility across major torch versions. If that's the case we may need to i) temporarily change tests to run off saved random data , ii) used the save data to make sure tests pass and then iii) revert to seeding and change the expected outputs.

UPDATE: These are all related to the NegSTOILoss. New results are correlated with the old but all a bit different. Looks more like some sort of precision error than a random number seeding issue. In fact, many tests that are using torch.rand are passing OK.

groadabike commented 1 year ago

The following torch-based regression tests are failing.

tests/predictor/test_torch_stoi.py ....FFFF.......
tests/regression/test_engine_losses.py ..FF
tests/regression/test_mc_conv_tasnet.py .F
tests/regression/test_predictors.py Fs.

I believe this may be because the random numbers have changed (this hypothesis needs checking...). In which case either the seeding needs fixing or it may be that even with the correct seeding there is no way to maintain reproducibility across major torch versions. If that's the case we may need to i) temporarily change tests to run off saved random data , ii) used the save data to make sure tests pass and then iii) revert to seeding and change the expected outputs.

@jonbarker68 Indeed they don't guarantee reproducibility across Pytorch versions https://pytorch.org/docs/stable/notes/randomness.html

In terms of the random seed, Pytorch 1.13 and 2.0 seem to be returning the same values. PyTorch 1.13 - Python 3.10.9 image

PyTorch 2.0 - Python 3.11.0 image

Perhaps, one solution is to set the deterministic_algorithm to TRUE if is not already https://pytorch.org/docs/stable/notes/randomness.html#avoiding-nondeterministic-algorithms

jonbarker68 commented 1 year ago

Perhaps, one solution is to set the deterministic_algorithm to TRUE if is not already https://pytorch.org/docs/stable/notes/randomness.html#avoiding-nondeterministic-algorithms

@groadabike Yes, on further investigation the issue does not appear to be the random numbers. I expect it's more likely to be numeric issues. e.g. we are getting small but significant differences in low STOI loss scores e.g., 0.0003 vs 0.0140. But we are comparing random signals that have no correlation. I expect these results are very sensitive to precision errors. I'll need to do a bit more digging.

jonbarker68 commented 1 year ago

@groadabike - the main problem turned out to be in torchaudio.transforms.Resample where there has been a change to the valid values for the 'resampling_method' parameter: before it was called "sinc_interpolation" now it is "sinc_interp_hann" ... but weirdly it didn't complain that the old value was no longer valid it just went and accepted and used some different method. That feels like a bug in the torchaudio code TBH. Now just one test failing.

codecov[bot] commented 1 year ago

Codecov Report

Merging #276 (f316410) into main (cb610f3) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #276   +/-   ##
=======================================
  Coverage   92.64%   92.64%           
=======================================
  Files          40       40           
  Lines        3781     3781           
=======================================
  Hits         3503     3503           
  Misses        278      278           
Impacted Files Coverage Δ
clarity/predictor/torch_stoi.py 94.05% <ø> (ø)
clarity/predictor/torch_msbg.py 96.64% <100.00%> (ø)
clarity/utils/audiogram.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jonbarker68 commented 1 year ago

The following test can take up to 5 mins

jonbarker68 commented 1 year ago

This PR could now be merged but doing so will require users to upgrade their torch version.