From 74b301895e1d4fb6ab2c25854f7c095d307371ae Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:04:40 +0100
Subject: [PATCH 01/16] Rename max_iters to cosine_schedule_period_iters

---
 casanovo/config.py              | 25 ++++++++-
 casanovo/config.yaml            | 89 ++++++++++++++++-----------------
 casanovo/denovo/model.py        | 51 ++++++++++---------
 casanovo/denovo/model_runner.py |  4 +-
 tests/conftest.py               |  2 +-
 5 files changed, 98 insertions(+), 73 deletions(-)

diff --git a/casanovo/config.py b/casanovo/config.py
index 817766ac..f1651b30 100644
--- a/casanovo/config.py
+++ b/casanovo/config.py
@@ -2,6 +2,7 @@
 
 import logging
 import shutil
+import warnings
 from pathlib import Path
 from typing import Optional, Dict, Callable, Tuple, Union
 
@@ -33,6 +34,11 @@ class Config:
     """
 
     _default_config = Path(__file__).parent / "config.yaml"
+    # FIXME: This contains deprecated config options to be removed in the next
+    #  major version update.
+    _config_deprecated_remap = dict(
+        max_iters="cosine_schedule_period_iters",
+    )
     _config_types = dict(
         random_seed=int,
         n_peaks=int,
@@ -56,7 +62,7 @@ class Config:
         tb_summarywriter=str,
         train_label_smoothing=float,
         warmup_iters=int,
-        max_iters=int,
+        cosine_schedule_period_iters=int,
         learning_rate=float,
         weight_decay=float,
         train_batch_size=int,
@@ -84,9 +90,22 @@ def __init__(self, config_file: Optional[str] = None):
         else:
             with Path(config_file).open() as f_in:
                 self._user_config = yaml.safe_load(f_in)
+                # Remap deprecated config entries.
+                for old, new in self._config_deprecated_remap.items():
+                    if old in self._user_config:
+                        self._user_config[new] = self._user_config.pop(old)
+                        warnings.warn(
+                            f"Deprecated config option '{old}' remapped to "
+                            f"'{new}'",
+                            DeprecationWarning,
+                        )
                 # Check for missing entries in config file.
                 config_missing = self._params.keys() - self._user_config.keys()
                 if len(config_missing) > 0:
+                    logger.error(
+                        "Missing expected config option(s): "
+                        f"{', '.join(config_missing)}"
+                    )
                     raise KeyError(
                         "Missing expected config option(s): "
                         f"{', '.join(config_missing)}"
@@ -94,6 +113,10 @@ def __init__(self, config_file: Optional[str] = None):
                 # Check for unrecognized config file entries.
                 config_unknown = self._user_config.keys() - self._params.keys()
                 if len(config_unknown) > 0:
+                    logger.error(
+                        "Unrecognized config option(s): "
+                        f"{', '.join(config_unknown)}"
+                    )
                     raise KeyError(
                         "Unrecognized config option(s): "
                         f"{', '.join(config_unknown)}"
diff --git a/casanovo/config.yaml b/casanovo/config.yaml
index 24bf4623..093c8105 100644
--- a/casanovo/config.yaml
+++ b/casanovo/config.yaml
@@ -4,30 +4,29 @@
 ###
 
 ###
-# The following parameters can be modified when running inference or
-# when fine-tuning an existing Casanovo model.
+# The following parameters can be modified when running inference or when
+# fine-tuning an existing Casanovo model.
 ###
 
-# Max absolute difference allowed with respect to observed precursor m/z
+# Max absolute difference allowed with respect to observed precursor m/z.
 # Predictions outside the tolerance range are assigned a negative peptide score.
 precursor_mass_tol: 50  # ppm
-# Isotopes to consider when comparing predicted and observed precursor m/z's
+# Isotopes to consider when comparing predicted and observed precursor m/z's.
 isotope_error_range: [0, 1]
-# The minimum length of predicted peptides
+# The minimum length of predicted peptides.
 min_peptide_len: 6
-# Number of spectra in one inference batch
+# Number of spectra in one inference batch.
 predict_batch_size: 1024
-# Number of beams used in beam search
+# Number of beams used in beam search.
 n_beams: 1
-# Number of PSMs for each spectrum
+# Number of PSMs for each spectrum.
 top_match: 1
 # The hardware accelerator to use. Must be one of:
-# "cpu", "gpu", "tpu", "ipu", "hpu", "mps", or "auto"
+# "cpu", "gpu", "tpu", "ipu", "hpu", "mps", or "auto".
 accelerator: "auto"
-# The devices to use. Can be set to a positive number int,
-# or the value -1 to indicate all available devices should be used,
-# If left empty, the appropriate number will be automatically
-# selected for automatic selected on the chosen accelerator.
+# The devices to use. Can be set to a positive number int, or the value -1 to
+# indicate all available devices should be used. If left empty, the appropriate
+# number will be automatically selected for based on the chosen accelerator.
 devices:
 
 ###
@@ -35,72 +34,72 @@ devices:
 # Casanovo model from scratch.
 ###
 
-# Random seed to ensure reproducible results
+# Random seed to ensure reproducible results.
 random_seed: 454
 
 # OUTPUT OPTIONS
-# Logging frequency in training steps
+# Logging frequency in training steps.
 n_log: 1
-# Tensorboard directory to use for keeping track of training metrics
+# Tensorboard directory to use for keeping track of training metrics.
 tb_summarywriter:
-# Save the top k model checkpoints during training. -1 saves all, and
-# leaving this field empty saves none.
+# Save the top k model checkpoints during training. -1 saves all, and leaving
+# this field empty saves none.
 save_top_k: 5
-# Path to saved checkpoints
+# Path to saved checkpoints.
 model_save_folder_path: ""
-# Model validation and checkpointing frequency in training steps
+# Model validation and checkpointing frequency in training steps.
 val_check_interval: 50_000
 
 # SPECTRUM PROCESSING OPTIONS
-# Number of the most intense peaks to retain, any remaining peaks are discarded
+# Number of the most intense peaks to retain, any remaining peaks are discarded.
 n_peaks: 150
-# Min peak m/z allowed, peaks with smaller m/z are discarded
+# Min peak m/z allowed, peaks with smaller m/z are discarded.
 min_mz: 50.0
-# Max peak m/z allowed, peaks with larger m/z are discarded
+# Max peak m/z allowed, peaks with larger m/z are discarded.
 max_mz: 2500.0
-# Min peak intensity allowed, less intense peaks are discarded
+# Min peak intensity allowed, less intense peaks are discarded.
 min_intensity: 0.01
-# Max absolute m/z difference allowed when removing the precursor peak
+# Max absolute m/z difference allowed when removing the precursor peak.
 remove_precursor_tol: 2.0  # Da
-# Max precursor charge allowed, spectra with larger charge are skipped
+# Max precursor charge allowed, spectra with larger charge are skipped.
 max_charge: 10
 
 # MODEL ARCHITECTURE OPTIONS
-# Dimensionality of latent representations, i.e. peak embeddings
+# Dimensionality of latent representations, i.e. peak embeddings.
 dim_model: 512
-# Number of attention heads
+# Number of attention heads.
 n_head: 8
-# Dimensionality of fully connected layers
+# Dimensionality of fully connected layers.
 dim_feedforward: 1024
-# Number of transformer layers in spectrum encoder and peptide decoder
+# Number of transformer layers in spectrum encoder and peptide decoder.
 n_layers: 9
-# Dropout rate for model weights
+# Dropout rate for model weights.
 dropout: 0.0
-# Number of dimensions to use for encoding peak intensity
-# Projected up to ``dim_model`` by default and summed with the peak m/z encoding
+# Number of dimensions to use for encoding peak intensity.
+# Projected up to `dim_model` by default and summed with the peak m/z encoding.
 dim_intensity:
-# Max decoded peptide length
+# Max decoded peptide length.
 max_length: 100
-# Number of warmup iterations for learning rate scheduler
+# The number of iterations for the linear warm-up of the learning rate.
 warmup_iters: 100_000
-# Max number of iterations for learning rate scheduler
-max_iters: 600_000
-# Learning rate for weight updates during training
+# The number of iterations for the cosine period of the learning rate.
+cosine_schedule_period_iters: 600_000
+# Learning rate for weight updates during training.
 learning_rate: 5e-4
-# Regularization term for weight updates
+# Regularization term for weight updates.
 weight_decay: 1e-5
-# Amount of label smoothing when computing the training loss
+# Amount of label smoothing when computing the training loss.
 train_label_smoothing: 0.01
 
 # TRAINING/INFERENCE OPTIONS
-# Number of spectra in one training batch
+# Number of spectra in one training batch.
 train_batch_size: 32
-# Max number of training epochs
+# Max number of training epochs.
 max_epochs: 30
-# Number of validation steps to run before training begins
+# Number of validation steps to run before training begins.
 num_sanity_val_steps: 0
-# Calculate peptide and amino acid precision during training. this
-# is expensive, so we recommend against it.
+# Calculate peptide and amino acid precision during training.
+# This is expensive, so we recommend against it.
 calculate_precision: False
 
 # AMINO ACID AND MODIFICATION VOCABULARY
diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index 9ea9cb23..57790144 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -46,7 +46,7 @@ class Spec2Pep(pl.LightningModule, ModelMixin):
         linear layer, then summed with the m/z encoding for each peak.
     max_length : int
         The maximum peptide length to decode.
-    residues: Union[Dict[str, float], str]
+    residues : Union[Dict[str, float], str]
         The amino acid dictionary and their masses. By default ("canonical) this
         is only the 20 canonical amino acids, with cysteine carbamidomethylated.
         If "massivekb", this dictionary will include the modifications found in
@@ -65,24 +65,24 @@ class Spec2Pep(pl.LightningModule, ModelMixin):
         < precursor_mass_tol`
     min_peptide_len : int
         The minimum length of predicted peptides.
-    n_beams: int
+    n_beams : int
         Number of beams used during beam search decoding.
-    top_match: int
+    top_match : int
         Number of PSMs to return for each spectrum.
     n_log : int
         The number of epochs to wait between logging messages.
-    tb_summarywriter: Optional[str]
+    tb_summarywriter : Optional[str]
         Folder path to record performance metrics during training. If ``None``,
         don't use a ``SummaryWriter``.
-    train_label_smoothing: float
+    train_label_smoothing : float
         Smoothing factor when calculating the training loss.
-    warmup_iters: int
-        The number of warm up iterations for the learning rate scheduler.
-    max_iters: int
-        The total number of iterations for the learning rate scheduler.
-    out_writer: Optional[str]
+    warmup_iters : int
+        The number of iterations for the linear warm-up of the learning rate.
+    cosine_schedule_period_iters : int
+        The number of iterations for the cosine period of the learning rate.
+    out_writer : Optional[str]
         The output writer for the prediction results.
-    calculate_precision: bool
+    calculate_precision : bool
         Calculate the validation set precision during training.
         This is expensive.
     **kwargs : Dict
@@ -111,7 +111,7 @@ def __init__(
         ] = None,
         train_label_smoothing: float = 0.01,
         warmup_iters: int = 100_000,
-        max_iters: int = 600_000,
+        cosine_schedule_period_iters: int = 600_000,
         out_writer: Optional[ms_io.MztabWriter] = None,
         calculate_precision: bool = False,
         **kwargs: Dict,
@@ -144,7 +144,7 @@ def __init__(
         self.val_celoss = torch.nn.CrossEntropyLoss(ignore_index=0)
         # Optimizer settings.
         self.warmup_iters = warmup_iters
-        self.max_iters = max_iters
+        self.cosine_schedule_period_iters = cosine_schedule_period_iters
         self.opt_kwargs = kwargs
 
         # Data properties.
@@ -960,29 +960,32 @@ def configure_optimizers(
         optimizer = torch.optim.Adam(self.parameters(), **self.opt_kwargs)
         # Apply learning rate scheduler per step.
         lr_scheduler = CosineWarmupScheduler(
-            optimizer, warmup=self.warmup_iters, max_iters=self.max_iters
+            optimizer, self.warmup_iters, self.cosine_schedule_period_iters
         )
         return [optimizer], {"scheduler": lr_scheduler, "interval": "step"}
 
 
 class CosineWarmupScheduler(torch.optim.lr_scheduler._LRScheduler):
     """
-    Learning rate scheduler with linear warm up followed by cosine shaped decay.
+    Learning rate scheduler with linear warm-up followed by cosine shaped decay.
 
     Parameters
     ----------
     optimizer : torch.optim.Optimizer
         Optimizer object.
-    warmup : int
-        The number of warm up iterations.
-    max_iters : torch.optim
-        The total number of iterations.
+    warmup_iters : int
+        The number of iterations for the linear warm-up of the learning rate.
+    cosine_schedule_period_iters : int
+        The number of iterations for the cosine period of the learning rate.
     """
 
     def __init__(
-        self, optimizer: torch.optim.Optimizer, warmup: int, max_iters: int
+        self, optimizer: torch.optim.Optimizer,
+        warmup_iters: int,
+        cosine_schedule_period_iters: int,
     ):
-        self.warmup, self.max_iters = warmup, max_iters
+        self.warmup_iters = warmup_iters
+        self.cosine_schedule_period_iters = cosine_schedule_period_iters
         super().__init__(optimizer)
 
     def get_lr(self):
@@ -990,9 +993,9 @@ def get_lr(self):
         return [base_lr * lr_factor for base_lr in self.base_lrs]
 
     def get_lr_factor(self, epoch):
-        lr_factor = 0.5 * (1 + np.cos(np.pi * epoch / self.max_iters))
-        if epoch <= self.warmup:
-            lr_factor *= epoch / self.warmup
+        lr_factor = 0.5 * (1 + np.cos(np.pi * epoch / self.cosine_schedule_period_iters))
+        if epoch <= self.warmup_iters:
+            lr_factor *= epoch / self.warmup_iters
         return lr_factor
 
 
diff --git a/casanovo/denovo/model_runner.py b/casanovo/denovo/model_runner.py
index 3253419a..55a3e69c 100644
--- a/casanovo/denovo/model_runner.py
+++ b/casanovo/denovo/model_runner.py
@@ -226,7 +226,7 @@ def initialize_model(self, train: bool) -> None:
             tb_summarywriter=self.config.tb_summarywriter,
             train_label_smoothing=self.config.train_label_smoothing,
             warmup_iters=self.config.warmup_iters,
-            max_iters=self.config.max_iters,
+            cosine_schedule_period_iters=self.config.cosine_schedule_period_iters,
             lr=self.config.learning_rate,
             weight_decay=self.config.weight_decay,
             out_writer=self.writer,
@@ -245,7 +245,7 @@ def initialize_model(self, train: bool) -> None:
             tb_summarywriter=self.config.tb_summarywriter,
             train_label_smoothing=self.config.train_label_smoothing,
             warmup_iters=self.config.warmup_iters,
-            max_iters=self.config.max_iters,
+            cosine_schedule_period_iters=self.config.cosine_schedule_period_iters,
             lr=self.config.learning_rate,
             weight_decay=self.config.weight_decay,
             out_writer=self.writer,
diff --git a/tests/conftest.py b/tests/conftest.py
index 3345824e..02a6d0f2 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -193,7 +193,7 @@ def tiny_config(tmp_path):
         "n_layers": 1,
         "train_label_smoothing": 0.01,
         "warmup_iters": 1,
-        "max_iters": 1,
+        "cosine_schedule_period_iters": 1,
         "max_epochs": 20,
         "val_check_interval": 1,
         "model_save_folder_path": str(tmp_path),

From 164d7aedb6bec2942f2390f7ad5b54589fb0d4b8 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:14:17 +0100
Subject: [PATCH 02/16] Add deprecated config option unit test

---
 tests/unit_tests/test_config.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py
index 1924d122..fbe10eee 100644
--- a/tests/unit_tests/test_config.py
+++ b/tests/unit_tests/test_config.py
@@ -37,3 +37,15 @@ def test_override(tmp_path, tiny_config):
 
     with pytest.raises(KeyError):
         Config(filename)
+
+
+def test_deprecated(tmp_path, tiny_config):
+    filename = str(tmp_path / "config_deprecated.yml")
+    with open(tiny_config, "r") as f_in, open(filename, "w") as f_out:
+        cfg = yaml.safe_load(f_in)
+        # Insert deprecated config option.
+        cfg["max_iters"] = 1
+        yaml.safe_dump(cfg, f_out)
+
+    with pytest.warns(DeprecationWarning):
+        Config(filename)

From dfd971622d3721f07c7a1802c2ce4b5ffa5f2943 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:16:16 +0100
Subject: [PATCH 03/16] Fix missed rename

---
 tests/unit_tests/test_runner.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index efaceb6b..1cc8eeba 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -62,7 +62,7 @@ def test_save_and_load_weights(tmp_path, mgf_small, tiny_config):
     other_config = Config(tiny_config)
     other_config.n_layers = 50  # lol
     other_config.n_beams = 12
-    other_config.max_iters = 2
+    other_config.cosine_schedule_period_iters = 2
     with torch.device("meta"):
         # Now load the weights into a new model
         # The device should be meta for all the weights.
@@ -72,7 +72,7 @@ def test_save_and_load_weights(tmp_path, mgf_small, tiny_config):
     obs_layers = runner.model.encoder.transformer_encoder.num_layers
     assert obs_layers == 1  # Match the original arch.
     assert runner.model.n_beams == 12  # Match the config
-    assert runner.model.max_iters == 2  # Match the config
+    assert runner.model.cosine_schedule_period_iters == 2  # Match the config
     assert next(runner.model.parameters()).device == torch.device("meta")
 
     # If the Trainer correctly moves the weights to the accelerator,

From 16bd9e426cb8b40a71e91ed0002562f825e2fb40 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:20:13 +0100
Subject: [PATCH 04/16] Proper linting

---
 casanovo/denovo/model.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index 57790144..9ef65f3c 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -980,7 +980,8 @@ class CosineWarmupScheduler(torch.optim.lr_scheduler._LRScheduler):
     """
 
     def __init__(
-        self, optimizer: torch.optim.Optimizer,
+        self,
+        optimizer: torch.optim.Optimizer,
         warmup_iters: int,
         cosine_schedule_period_iters: int,
     ):
@@ -993,7 +994,9 @@ def get_lr(self):
         return [base_lr * lr_factor for base_lr in self.base_lrs]
 
     def get_lr_factor(self, epoch):
-        lr_factor = 0.5 * (1 + np.cos(np.pi * epoch / self.cosine_schedule_period_iters))
+        lr_factor = 0.5 * (
+            1 + np.cos(np.pi * epoch / self.cosine_schedule_period_iters)
+        )
         if epoch <= self.warmup_iters:
             lr_factor *= epoch / self.warmup_iters
         return lr_factor

From 78408178375336f91370f2713765f6b15b41ee24 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:20:22 +0100
Subject: [PATCH 05/16] Remove unnecessary logging

---
 casanovo/config.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/casanovo/config.py b/casanovo/config.py
index f1651b30..163dd200 100644
--- a/casanovo/config.py
+++ b/casanovo/config.py
@@ -102,10 +102,6 @@ def __init__(self, config_file: Optional[str] = None):
                 # Check for missing entries in config file.
                 config_missing = self._params.keys() - self._user_config.keys()
                 if len(config_missing) > 0:
-                    logger.error(
-                        "Missing expected config option(s): "
-                        f"{', '.join(config_missing)}"
-                    )
                     raise KeyError(
                         "Missing expected config option(s): "
                         f"{', '.join(config_missing)}"
@@ -113,10 +109,6 @@ def __init__(self, config_file: Optional[str] = None):
                 # Check for unrecognized config file entries.
                 config_unknown = self._user_config.keys() - self._params.keys()
                 if len(config_unknown) > 0:
-                    logger.error(
-                        "Unrecognized config option(s): "
-                        f"{', '.join(config_unknown)}"
-                    )
                     raise KeyError(
                         "Unrecognized config option(s): "
                         f"{', '.join(config_unknown)}"

From 9d4d80fc4d654adf49de13c4205094318a08406c Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:47:42 +0100
Subject: [PATCH 06/16] Test that checkpoints with deprecated config options
 can be loaded

---
 tests/unit_tests/test_runner.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index 1cc8eeba..f7266bc5 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -99,6 +99,28 @@ def test_save_and_load_weights(tmp_path, mgf_small, tiny_config):
         runner.evaluate([mgf_small])
 
 
+def test_save_and_load_weights_deprecated(tmp_path, mgf_small, tiny_config):
+    """Test saving and loading weights with deprecated config options."""
+    config = Config(tiny_config)
+    config.max_epochs = 1
+    config.cosine_schedule_period_iters = 5
+    ckpt = tmp_path / "test.ckpt"
+
+    with ModelRunner(config=config) as runner:
+        runner.train([mgf_small], [mgf_small])
+        runner.trainer.save_checkpoint(ckpt)
+
+    # Replace the new config option with the deprecated one.
+    ckpt_data = torch.load(ckpt)
+    ckpt_data["hyper_parameters"]["max_iters"] = 5
+    del ckpt_data["hyper_parameters"]["cosine_schedule_period_iters"]
+    torch.save(ckpt_data, str(tmp_path / "test.ckpt"))
+
+    with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
+        runner.initialize_model(train=False)
+        assert runner.model.cosine_schedule_period_iters == 5
+
+
 def test_calculate_precision(tmp_path, mgf_small, tiny_config):
     """Test that this parameter is working correctly."""
     config = Config(tiny_config)

From dc6865bc6e381071992b406a7cc9e50cd01acc4d Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Tue, 20 Feb 2024 19:49:10 +0100
Subject: [PATCH 07/16] Minor change

---
 tests/unit_tests/test_runner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index f7266bc5..ba3d2e41 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -114,7 +114,7 @@ def test_save_and_load_weights_deprecated(tmp_path, mgf_small, tiny_config):
     ckpt_data = torch.load(ckpt)
     ckpt_data["hyper_parameters"]["max_iters"] = 5
     del ckpt_data["hyper_parameters"]["cosine_schedule_period_iters"]
-    torch.save(ckpt_data, str(tmp_path / "test.ckpt"))
+    torch.save(ckpt_data, str(ckpt))
 
     with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
         runner.initialize_model(train=False)

From ff827c63e31056067a5545e3f9c0665c0b6a21a7 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Wed, 21 Feb 2024 08:25:29 +0100
Subject: [PATCH 08/16] Add test for fine-tuning with deprecated config options

---
 tests/unit_tests/test_runner.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index ba3d2e41..52c34886 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -116,9 +116,13 @@ def test_save_and_load_weights_deprecated(tmp_path, mgf_small, tiny_config):
     del ckpt_data["hyper_parameters"]["cosine_schedule_period_iters"]
     torch.save(ckpt_data, str(ckpt))
 
+    # Inference.
     with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
         runner.initialize_model(train=False)
         assert runner.model.cosine_schedule_period_iters == 5
+    # Fine-tuning.
+    with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
+        runner.train([mgf_small], [mgf_small])
 
 
 def test_calculate_precision(tmp_path, mgf_small, tiny_config):

From 2da0d1c5ef94c476c77485edd64b88f993323128 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Wed, 21 Feb 2024 08:50:55 +0100
Subject: [PATCH 09/16] Remove deprecated hyperparameters during model loading

---
 casanovo/denovo/model.py        | 6 ++++++
 casanovo/denovo/model_runner.py | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index 9ef65f3c..61a76374 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -18,6 +18,8 @@
 
 logger = logging.getLogger("casanovo")
 
+_kwargs_deprecated = ["max_iters"]
+
 
 class Spec2Pep(pl.LightningModule, ModelMixin):
     """
@@ -145,6 +147,10 @@ def __init__(
         # Optimizer settings.
         self.warmup_iters = warmup_iters
         self.cosine_schedule_period_iters = cosine_schedule_period_iters
+        # `kwargs` will contain additional arguments as well as unrecognized
+        # arguments, including deprecated ones. Remove the deprecated ones.
+        for k in _kwargs_deprecated:
+            kwargs.pop(k, None)
         self.opt_kwargs = kwargs
 
         # Data properties.
diff --git a/casanovo/denovo/model_runner.py b/casanovo/denovo/model_runner.py
index 55a3e69c..4bd2165e 100644
--- a/casanovo/denovo/model_runner.py
+++ b/casanovo/denovo/model_runner.py
@@ -204,8 +204,8 @@ def initialize_model(self, train: bool) -> None:
         Parameters
         ----------
         train : bool
-            Determines whether to set the model up for model training
-            or evaluation / inference.
+            Determines whether to set the model up for model training or
+            evaluation / inference.
         """
         model_params = dict(
             dim_model=self.config.dim_model,
@@ -233,7 +233,7 @@ def initialize_model(self, train: bool) -> None:
             calculate_precision=self.config.calculate_precision,
         )
 
-        # Reconfigurable non-architecture related parameters for a loaded model
+        # Reconfigurable non-architecture related parameters for a loaded model.
         loaded_model_params = dict(
             max_length=self.config.max_length,
             precursor_mass_tol=self.config.precursor_mass_tol,
@@ -300,7 +300,7 @@ def initialize_model(self, train: bool) -> None:
             except RuntimeError:
                 raise RuntimeError(
                     "Weights file incompatible with the current version of "
-                    "Casanovo. "
+                    "Casanovo."
                 )
 
     def initialize_data_module(

From be897495e367ef2b535c0170d58083353623c6f9 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Wed, 21 Feb 2024 08:55:09 +0100
Subject: [PATCH 10/16] Include deprecated hyperparameter warning

---
 casanovo/denovo/model.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index 61a76374..dc082bfb 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -3,6 +3,7 @@
 import collections
 import heapq
 import logging
+import warnings
 from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
 
 import depthcharge.masses
@@ -151,6 +152,10 @@ def __init__(
         # arguments, including deprecated ones. Remove the deprecated ones.
         for k in _kwargs_deprecated:
             kwargs.pop(k, None)
+            warnings.warn(
+                f"Deprecated hyperparameter '{k}' removed from the model.",
+                DeprecationWarning,
+            )
         self.opt_kwargs = kwargs
 
         # Data properties.

From a950ee69c695a3ef09d78138321de95cdd332cc7 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Wed, 21 Feb 2024 08:57:59 +0100
Subject: [PATCH 11/16] Test whether the warning is issued

---
 tests/unit_tests/test_runner.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index 52c34886..2e6b1c72 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -122,7 +122,8 @@ def test_save_and_load_weights_deprecated(tmp_path, mgf_small, tiny_config):
         assert runner.model.cosine_schedule_period_iters == 5
     # Fine-tuning.
     with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
-        runner.train([mgf_small], [mgf_small])
+        with pytest.warns(DeprecationWarning):
+            runner.train([mgf_small], [mgf_small])
 
 
 def test_calculate_precision(tmp_path, mgf_small, tiny_config):

From fad4f1faf0a9b1d8867010c7337dfc87b6ab670c Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Wed, 21 Feb 2024 09:01:20 +0100
Subject: [PATCH 12/16] Verify that the deprecated option is removed

---
 tests/unit_tests/test_runner.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/unit_tests/test_runner.py b/tests/unit_tests/test_runner.py
index 2e6b1c72..7febf3f7 100644
--- a/tests/unit_tests/test_runner.py
+++ b/tests/unit_tests/test_runner.py
@@ -124,6 +124,7 @@ def test_save_and_load_weights_deprecated(tmp_path, mgf_small, tiny_config):
     with ModelRunner(config=config, model_filename=str(ckpt)) as runner:
         with pytest.warns(DeprecationWarning):
             runner.train([mgf_small], [mgf_small])
+            assert "max_iters" not in runner.model.opt_kwargs
 
 
 def test_calculate_precision(tmp_path, mgf_small, tiny_config):

From 25c55a18823b965cd180afc6e9d15bfd4b085d62 Mon Sep 17 00:00:00 2001
From: melihyilmaz <yilmazmelih97@gmail.com>
Date: Wed, 21 Feb 2024 11:39:39 -0800
Subject: [PATCH 13/16] Fix comments

---
 casanovo/config.yaml     | 2 +-
 casanovo/denovo/model.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/casanovo/config.yaml b/casanovo/config.yaml
index 093c8105..c7186ff7 100644
--- a/casanovo/config.yaml
+++ b/casanovo/config.yaml
@@ -82,7 +82,7 @@ dim_intensity:
 max_length: 100
 # The number of iterations for the linear warm-up of the learning rate.
 warmup_iters: 100_000
-# The number of iterations for the cosine period of the learning rate.
+# The number of iterations for the cosine half period of the learning rate.
 cosine_schedule_period_iters: 600_000
 # Learning rate for weight updates during training.
 learning_rate: 5e-4
diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index dc082bfb..dfb858e0 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -82,7 +82,7 @@ class Spec2Pep(pl.LightningModule, ModelMixin):
     warmup_iters : int
         The number of iterations for the linear warm-up of the learning rate.
     cosine_schedule_period_iters : int
-        The number of iterations for the cosine period of the learning rate.
+        The number of iterations for the cosine half period of the learning rate.
     out_writer : Optional[str]
         The output writer for the prediction results.
     calculate_precision : bool
@@ -987,7 +987,7 @@ class CosineWarmupScheduler(torch.optim.lr_scheduler._LRScheduler):
     warmup_iters : int
         The number of iterations for the linear warm-up of the learning rate.
     cosine_schedule_period_iters : int
-        The number of iterations for the cosine period of the learning rate.
+        The number of iterations for the cosine half period of the learning rate.
     """
 
     def __init__(

From 589f72b94237d235081b375a102a65ff4961ba11 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Thu, 22 Feb 2024 09:47:49 +0100
Subject: [PATCH 14/16] Avoid defining deprecated options twice

---
 casanovo/config.py       | 14 ++++++++------
 casanovo/denovo/model.py |  5 ++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/casanovo/config.py b/casanovo/config.py
index 163dd200..2ec561fd 100644
--- a/casanovo/config.py
+++ b/casanovo/config.py
@@ -13,6 +13,13 @@
 logger = logging.getLogger("casanovo")
 
 
+# FIXME: This contains deprecated config options to be removed in the next major
+#  version update.
+_config_deprecated = dict(
+    max_iters="cosine_schedule_period_iters",
+)
+
+
 class Config:
     """The Casanovo configuration options.
 
@@ -34,11 +41,6 @@ class Config:
     """
 
     _default_config = Path(__file__).parent / "config.yaml"
-    # FIXME: This contains deprecated config options to be removed in the next
-    #  major version update.
-    _config_deprecated_remap = dict(
-        max_iters="cosine_schedule_period_iters",
-    )
     _config_types = dict(
         random_seed=int,
         n_peaks=int,
@@ -91,7 +93,7 @@ def __init__(self, config_file: Optional[str] = None):
             with Path(config_file).open() as f_in:
                 self._user_config = yaml.safe_load(f_in)
                 # Remap deprecated config entries.
-                for old, new in self._config_deprecated_remap.items():
+                for old, new in _config_deprecated.items():
                     if old in self._user_config:
                         self._user_config[new] = self._user_config.pop(old)
                         warnings.warn(
diff --git a/casanovo/denovo/model.py b/casanovo/denovo/model.py
index dfb858e0..50d43047 100644
--- a/casanovo/denovo/model.py
+++ b/casanovo/denovo/model.py
@@ -15,12 +15,11 @@
 from depthcharge.components import ModelMixin, PeptideDecoder, SpectrumEncoder
 
 from . import evaluate
+from .. import config
 from ..data import ms_io
 
 logger = logging.getLogger("casanovo")
 
-_kwargs_deprecated = ["max_iters"]
-
 
 class Spec2Pep(pl.LightningModule, ModelMixin):
     """
@@ -150,7 +149,7 @@ def __init__(
         self.cosine_schedule_period_iters = cosine_schedule_period_iters
         # `kwargs` will contain additional arguments as well as unrecognized
         # arguments, including deprecated ones. Remove the deprecated ones.
-        for k in _kwargs_deprecated:
+        for k in config._config_deprecated:
             kwargs.pop(k, None)
             warnings.warn(
                 f"Deprecated hyperparameter '{k}' removed from the model.",

From a04ce47554b8ce7383266c6cad8a4503370434b3 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Thu, 22 Feb 2024 09:51:54 +0100
Subject: [PATCH 15/16] Remap previous renamed config option
 `every_n_train_steps`

---
 casanovo/config.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/casanovo/config.py b/casanovo/config.py
index 2ec561fd..792da35a 100644
--- a/casanovo/config.py
+++ b/casanovo/config.py
@@ -16,6 +16,7 @@
 # FIXME: This contains deprecated config options to be removed in the next major
 #  version update.
 _config_deprecated = dict(
+    every_n_train_steps="val_check_interval",
     max_iters="cosine_schedule_period_iters",
 )
 

From 0c0ccaab547e1369dd6ac71d6baa4976eaa5c012 Mon Sep 17 00:00:00 2001
From: Wout Bittremieux <wout@bittremieux.be>
Date: Thu, 22 Feb 2024 09:52:01 +0100
Subject: [PATCH 16/16] Update changelog

---
 CHANGELOG.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a9f5f939..040e87e3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased]
 
+### Added
+
+- A deprecation warning will be issued when depecrated config options are used in the config file or in the model weights file.
+
+### Changed
+
+- Config option `max_iters` has been renamed to `cosine_schedule_period_iters` to better reflect that it controls the number of iterations for the cosine half period of the learning rate.
+
 ## [4.1.0] - 2024-02-16
 
 ### Changed