Adding checkpoint and config version

[SamuelLarkin]

PR Goal?

Add a version number in a saved model's checkpoint and a version to EveryVoice's config files.

Fixes?

#559

Feedback sought?

Merge approval

Priority?

high

Tests added?

yes

How to test?

python -m unittest \
  everyvoice.tests.test_model.TestLoadingConfig \
  everyvoice.tests.test_model.TestLoadingModel \
  everyvoice.tests.test_cli

Confidence?

good

Version change?

yes for the schemas

Related PRs?

EveryVoiceTTS/FastSpeech2_lightning#92
EveryVoiceTTS/HiFiGAN_iSTFT_lightning#38
EveryVoiceTTS/DeepForcedAligner#28

[semanticdiff-com]

Review changes with  

Changed Files

File	Status
everyvoice/tests/test_cli.py	95% smaller
everyvoice/_version.py	60% smaller
everyvoice/.schema/everyvoice-aligner-0.2.json	0% smaller
everyvoice/.schema/everyvoice-shared-data-0.2.json	0% smaller
everyvoice/.schema/everyvoice-shared-text-0.2.json	0% smaller
everyvoice/.schema/everyvoice-spec-to-wav-0.2.json	0% smaller
everyvoice/.schema/everyvoice-text-to-spec-0.2.json	0% smaller
everyvoice/.schema/everyvoice-text-to-wav-0.2.json	0% smaller
everyvoice/model/aligner/DeepForcedAligner	0% smaller
everyvoice/model/feature_prediction/FastSpeech2_lightning	0% smaller
everyvoice/model/vocoder/HiFiGAN_iSTFT_lightning	0% smaller
everyvoice/tests/test_model.py	0% smaller

[github-actions]

CLI load time: 0:00.29
Pull Request HEAD: 1cc510dc84f5cd73d1b075f3c12554186d26a6e8
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.28%. Comparing base (49de253) to head (1cc510d).
Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #565   +/-   ##
=======================================
  Coverage   76.28%   76.28%           
=======================================
  Files          46       46           
  Lines        3450     3450           
  Branches      472      472           
=======================================
  Hits         2632     2632           
  Misses        713      713           
  Partials      105      105           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joanise]

Darn, this is going to require a schema update, which normally means a minor version bump.

@roedoejet are the schemas published yet? If so, we'll need a minor bump to 0.2.0a0 instead of continuing with 0.1.0a4. Or maybe 0.2.0a4 to continue with our milestone naming? That's definitely not typical progression, but hey! sometimes we're stuck with stuff like that.

[joanise]

Version bump or not, I still think this is the right thing to do.

[roedoejet]

looks good to me! just a couple small things about the error messages

[roedoejet]

I'm surprised that "BAD_VERSION" causes an error message that says "your model was created with a newer version of EveryVoice" - isn't "BAD_VERSION" just an unknown version? I would rather see messaging that indicates whether the provided version is newer/older or unknown.

[roedoejet]

Darn, this is going to require a schema update, which normally means a minor version bump.

@roedoejet are the schemas published yet? If so, we'll need a minor bump to 0.2.0a0 instead of continuing with 0.1.0a4. Or maybe 0.2.0a4 to continue with our milestone naming? That's definitely not typical progression, but hey! sometimes we're stuck with stuff like that.

right - yea if we release this as 0.1.0a4 then the schema will be broken, but I don't think it's that big of a deal actually. In the case where people don't change their configs to accommodate the new version key, the schema checker won't care, and if they create a new config with the version number, it'll just underline that value (if they have the yaml extension installed) but otherwise it shouldn't cause any problems. And, if they see the underline and remove the version number, it won't actually matter since we handle it right now with this PR anyways. Mind you, maybe we should get in the practise of bumping the schemas like this, in which case I think we should go to 0.2.0a0 and I'll change the milestone names.

[joanise]

Hum, I added logging in each check_and_upgrade_checkpoint() function just to see what's going on, and I get my expected logging line from everyvoice.model.feature_prediction.FastSpeech2_lightning.fs2.model:check_and_upgrade_checkpoint:260 but I can't get the Vocoder loading to seem to call its own check_and_upgrade_checkpoint function.

I don't know if I'm doing something wrong, or if there's yet another model variant that needs the same code change. We can rubber duck when you are available.

[joanise]

but I can't get the Vocoder loading to seem to call its own check_and_upgrade_checkpoint function.

I tried with a Vocoder I trained myself, and with /gpfs/fs3c/nrc/dt/sgile/models/hifigan/hifigan_universal_v1_everyvoice.ckpt, and in both cases HiFiGANGenerator.check_and_upgrade_checkpoint() does not appear to get called.

I'm testing with everyvoice synthesize from-text which has to load both an fs2 and an hfgl model, if I understand things correctly:

everyvoice synthesize from-text -t "My test." logs_and_checkpoints/FeaturePredictionExperiment/base/checkpoints/last.ckpt --vocoder-path /gpfs/fs3c/nrc/dt/sgile/models/hifigan/hifigan_universal_v1_everyvoice.ckpt
everyvoice synthesize from-text -t "My test." logs_and_checkpoints/FeaturePredictionExperiment/base/checkpoints/last.ckpt --vocoder-path logs_and_checkpoints/VocoderExperiment/base/checkpoints/last.ckpt

[joanise]

Looks good, thanks for your patience with all the change requests!