search

Noble-Lab / casanovo

De Novo Mass Spectrometry Peptide Sequencing with a Transformer Model
https://casanovo.readthedocs.io
Apache License 2.0
102 stars 37 forks source link

Rename max_iters to cosine_schedule_period_iters #300

Closed bittremieux closed 7 months ago

bittremieux commented 7 months ago

Fixes #242.

cosine_schedule_period_iters better reflects what this config option does, and has correspondingly been renamed.

The config loader checks whether max_iters is specified and automatically remaps it to cosine_schedule_period_iters, while warning the user about this renaming.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (83a1ce4) 89.64% compared to head (0c0ccaa) 89.77%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #300 +/- ## ========================================== + Coverage 89.64% 89.77% +0.13% ========================================== Files 12 12 Lines 917 929 +12 ========================================== + Hits 822 834 +12 Misses 95 95 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bittremieux commented 7 months ago

Ideally we fix it in the code as well. Otherwise other people who try fine-tuning will have the same problem.

I added a unit test to check this and added a fix to remove unrecognized hyperparameters during model loading. This seems to work, but I find it a bit less elegant than the previous fix, because now we're changing config options both in Config and in Spec2Pep. Do you see any alternative solutions @melihyilmaz @wfondrie?