Repository: pytorch/ignite

Issue:
Fix CI
CI is broken in `test_create_lr_scheduler_with_warmup` with `pytorch-nightly`.
https://travis-ci.org/pytorch/ignite/jobs/525001845

Problem is related to the breaking change in pytorch-nightly : https://github.com/pytorch/pytorch/pull/7889
and https://github.com/pytorch/pytorch/pull/14010

Discussion:

Patch:
diff --git a/ignite/contrib/handlers/param_scheduler.py b/ignite/contrib/handlers/param_scheduler.py
index 43ea710e..fc2af497 100644
--- a/ignite/contrib/handlers/param_scheduler.py
+++ b/ignite/contrib/handlers/param_scheduler.py
@@ -117,7 +117,7 @@ class CyclicalScheduler(ParamScheduler):
         param_name (str): name of optimizer's parameter to update.
         start_value (float): value at start of cycle.
         end_value (float): value at the middle of the cycle.
-        cycle_size (int): length of cycle.
+        cycle_size (int): length of cycle, value should be larger than 1.
         cycle_mult (float, optional): ratio by which to change the cycle_size.
             at the end of each cycle (default=1.0).
         start_value_mult (float, optional): ratio by which to change the start value at the
@@ -155,6 +155,10 @@ class CyclicalScheduler(ParamScheduler):
         self.start_value_mult = start_value_mult
         self.end_value_mult = end_value_mult
 
+        if self.cycle_size < 2:
+            raise ValueError("Argument cycle_size should be positive and larger than 1, "
+                             "but given {}".format(cycle_size))
+
     def __call__(self, engine, name=None):
         if self.event_index != 0 and self.event_index % self.cycle_size == 0:
             self.event_index = 0
@@ -304,6 +308,7 @@ class ConcatScheduler(ParamScheduler):
         # starts an annealing schedule from 0.5 to 0.01 over 60 iterations.
         # The annealing cycles are repeated indefinitely.
         #
+
     """
 
     def __init__(self, schedulers, durations, save_history=False):
@@ -368,7 +373,7 @@ class ConcatScheduler(ParamScheduler):
             s.save_history = value
 
     def get_param(self):
-        pass
+        return self._current_scheduler.get_param()
 
     @classmethod
     def simulate_values(cls, num_events, schedulers, durations, param_names=None, **kwargs):
@@ -390,7 +395,9 @@ class ConcatScheduler(ParamScheduler):
         output = []
 
         # Need to copy all schedulers otherwise unsafe
-        copy_schedulers = [_replicate_scheduler(s) for s in schedulers]
+        # Need to setup properly the copies of optimizer otherwise incorrect simulation values for pytorch >= 1.1.0
+        opt_copy_map = {id(s.optimizer_param_groups): deepcopy(s.optimizer_param_groups) for s in schedulers}
+        copy_schedulers = [_replicate_scheduler(s, opt_copy_map) for s in schedulers]
         scheduler = cls(copy_schedulers, durations, save_history=False)
         if param_names is None:
             param_names = [scheduler.param_name]
@@ -474,12 +481,16 @@ class LRScheduler(ParamScheduler):
         return values
 
     @staticmethod
-    def _replicate_lr_scheduler(lr_scheduler):
+    def _replicate_lr_scheduler(lr_scheduler, new_optimizer_param_groups=None):
+        if not isinstance(lr_scheduler, _LRScheduler):
+            raise TypeError("lr_scheduler should inherit of _LRScheduler")
         lr_scheduler_cls = lr_scheduler.__class__
         optimizer_cls = lr_scheduler.optimizer.__class__
         t = torch.zeros([1], requires_grad=True)
         dummy_optimizer = optimizer_cls([t], lr=0.1)
         dummy_optimizer.load_state_dict(lr_scheduler.optimizer.state_dict())
+        if new_optimizer_param_groups is not None:
+            dummy_optimizer.param_groups = new_optimizer_param_groups
         kwargs = lr_scheduler.state_dict()
         del kwargs['base_lrs']
         copy_lr_scheduler = lr_scheduler_cls(optimizer=dummy_optimizer, **kwargs)
@@ -671,13 +682,17 @@ class PiecewiseLinear(ParamScheduler):
         return start_value + (end_value - start_value) * (self.event_index - start_index) / (end_index - start_index)
 
 
-def _replicate_scheduler(scheduler, save_history=False):
+def _replicate_scheduler(scheduler, opt_copy_map, save_history=False):
     if isinstance(scheduler, LRScheduler):
-        return LRScheduler(LRScheduler._replicate_lr_scheduler(scheduler.lr_scheduler), save_history=save_history)
+        opt_param_groups = opt_copy_map[id(scheduler.optimizer_param_groups)]
+        return LRScheduler(LRScheduler._replicate_lr_scheduler(scheduler.lr_scheduler, opt_param_groups),
+                           save_history=save_history)
     elif isinstance(scheduler, ConcatScheduler):
-        copy_schedulers = [_replicate_scheduler(s, save_history=save_history) for s in scheduler.schedulers]
+        copy_schedulers = [_replicate_scheduler(s, opt_copy_map, save_history=save_history)
+                           for s in scheduler.schedulers]
         return ConcatScheduler(copy_schedulers, durations=scheduler.durations, save_history=save_history)
     else:
         new_scheduler = deepcopy(scheduler)
+        new_scheduler.optimizer_param_groups = opt_copy_map[id(scheduler.optimizer_param_groups)]
         new_scheduler.save_history = save_history
         return new_scheduler


Test patch:
diff --git a/tests/ignite/contrib/handlers/test_param_scheduler.py b/tests/ignite/contrib/handlers/test_param_scheduler.py
index 5daed7b8..9090f46d 100644
--- a/tests/ignite/contrib/handlers/test_param_scheduler.py
+++ b/tests/ignite/contrib/handlers/test_param_scheduler.py
@@ -11,6 +11,13 @@ from ignite.contrib.handlers.param_scheduler import ParamGroupScheduler, Piecewi
 
 
 def test_linear_scheduler():
+
+    with pytest.raises(ValueError):
+        LinearCyclicalScheduler({}, 'lr', 1, 0, cycle_size=0)
+
+    with pytest.raises(ValueError):
+        LinearCyclicalScheduler({}, 'lr', 1, 0, cycle_size=1)
+
     tensor = torch.zeros([1], requires_grad=True)
     optimizer = torch.optim.SGD([tensor], lr=0)
 
@@ -77,6 +84,27 @@ def test_linear_scheduler():
     ]))
 
 
+def test_linear_scheduler_cycle_size_two():
+    tensor = torch.zeros([1], requires_grad=True)
+    optimizer = torch.optim.SGD([tensor], lr=0)
+
+    scheduler = LinearCyclicalScheduler(optimizer, 'lr', 1, 0, cycle_size=2)
+    lrs = []
+
+    def save_lr(engine):
+        lrs.append(optimizer.param_groups[0]['lr'])
+
+    trainer = Engine(lambda engine, batch: None)
+    trainer.add_event_handler(Events.ITERATION_STARTED, scheduler)
+    trainer.add_event_handler(Events.ITERATION_COMPLETED, save_lr)
+    trainer.run([0] * 10, max_epochs=2)
+
+    assert lrs == list(map(pytest.approx, [
+        1.0, 0.0, 1.0, 0.0, 1.0, 0.0, 1.0, 0.0, 1.0, 0.0,
+        1.0, 0.0, 1.0, 0.0, 1.0, 0.0, 1.0, 0.0, 1.0, 0.0
+    ]))
+
+
 def test_cosine_annealing_scheduler():
     tensor = torch.zeros([1], requires_grad=True)
     optimizer = torch.optim.SGD([tensor], lr=0)
@@ -182,6 +210,51 @@ def test_concat_scheduler():
     _test(duration_vals_as_np_int=True)
 
 
+def test_concat_scheduler_two_linear():
+    tensor = torch.zeros([1], requires_grad=True)
+    optimizer = torch.optim.SGD([tensor], lr=0)
+
+    scheduler_1 = LinearCyclicalScheduler(optimizer, "lr", start_value=0.0, end_value=0.1, cycle_size=2)
+    scheduler_2 = LinearCyclicalScheduler(optimizer, "lr", start_value=0.2, end_value=1.0, cycle_size=2)
+
+    durations = [5, ]
+    concat_scheduler = ConcatScheduler(schedulers=[scheduler_1, scheduler_2],
+                                       durations=durations, save_history=True)
+
+    assert concat_scheduler.get_param() == 0.0
+
+    data = [0] * 10
+    max_epochs = 2
+    simulated_values = ConcatScheduler.simulate_values(num_events=len(data) * max_epochs,
+                                                       schedulers=[scheduler_1, scheduler_2],
+                                                       durations=durations)
+
+    lrs = []
+
+    def save_lr(engine):
+        lrs.append(optimizer.param_groups[0]['lr'])
+
+    trainer = Engine(lambda engine, batch: None)
+    trainer.add_event_handler(Events.ITERATION_STARTED, concat_scheduler)
+    trainer.add_event_handler(Events.ITERATION_COMPLETED, save_lr)
+    trainer.run(data, max_epochs=max_epochs)
+
+    assert lrs == list(map(pytest.approx, [
+        # first LinearCyclicalScheduler
+        0.0, 0.1, 0.0, 0.1, 0.0,
+        # second LinearCyclicalScheduler
+        0.2, 1.0, 0.2, 1.0, 0.2, 1.0, 0.2, 1.0,
+        0.2, 1.0, 0.2, 1.0, 0.2, 1.0, 0.2,
+    ]))
+
+    state_lrs = trainer.state.param_history['lr']
+    assert len(state_lrs) == len(lrs)
+    # Unpack singleton lists
+    assert [group[0] for group in state_lrs] == lrs
+
+    assert lrs == pytest.approx([v for i, v in simulated_values])
+
+
 def test_concat_scheduler_3_schedulers():
     tensor = torch.zeros([1], requires_grad=True)
     optimizer = torch.optim.SGD([tensor], lr=0)
@@ -274,8 +347,8 @@ def test_lr_scheduler():
     def _test(torch_lr_scheduler_cls, **kwargs):
 
         tensor = torch.zeros([1], requires_grad=True)
-        optimizer1 = torch.optim.SGD([tensor], lr=0.1)
-        optimizer2 = torch.optim.SGD([tensor], lr=0.1)
+        optimizer1 = torch.optim.SGD([tensor], lr=0.01)
+        optimizer2 = torch.optim.SGD([tensor], lr=0.01)
 
         torch_lr_scheduler1 = torch_lr_scheduler_cls(optimizer=optimizer1, **kwargs)
         torch_lr_scheduler2 = torch_lr_scheduler_cls(optimizer=optimizer2, **kwargs)
@@ -306,7 +379,7 @@ def test_lr_scheduler():
 
         assert lrs_true == pytest.approx(lrs)
 
-        optimizer3 = torch.optim.SGD([tensor], lr=0.1)
+        optimizer3 = torch.optim.SGD([tensor], lr=0.01)
         torch_lr_scheduler3 = torch_lr_scheduler_cls(optimizer=optimizer3, **kwargs)
 
         simulated_values = LRScheduler.simulate_values(num_events=len(data) * max_epochs,
@@ -318,7 +391,7 @@ def test_lr_scheduler():
 
     # test _replicate_lr_scheduler
     tensor = torch.zeros([1], requires_grad=True)
-    optimizer = torch.optim.SGD([tensor], lr=0.1)
+    optimizer = torch.optim.SGD([tensor], lr=0.01)
     lr_scheduler = torch.optim.lr_scheduler.ExponentialLR(optimizer=optimizer, gamma=0.78)
     init_lr_scheduler_state = dict(lr_scheduler.state_dict())
     copy_lr_scheduler = LRScheduler._replicate_lr_scheduler(lr_scheduler)
@@ -327,6 +400,9 @@ def test_lr_scheduler():
 
     assert copy_lr_scheduler.state_dict() == init_lr_scheduler_state
 
+    with pytest.raises(TypeError):
+        LRScheduler._replicate_lr_scheduler(12)
+
 
 def test_piecewiselinear_asserts():
 
@@ -491,12 +567,12 @@ def test_create_lr_scheduler_with_warmup():
         assert lrs == pytest.approx([v for i, v in simulated_values])
 
     t1 = torch.zeros([1], requires_grad=True)
-    optimizer = torch.optim.SGD([t1], lr=0.1)
+    optimizer = torch.optim.SGD([t1], lr=0.2)
     torch_lr_scheduler = torch.optim.lr_scheduler.ExponentialLR(optimizer=optimizer, gamma=0.98)
     _test(torch_lr_scheduler, optimizer)
 
     t1 = torch.zeros([1], requires_grad=True)
-    optimizer = torch.optim.SGD([t1], lr=0.1)
+    optimizer = torch.optim.SGD([t1], lr=0.2)
     lr_scheduler = LinearCyclicalScheduler(optimizer=optimizer, param_name='lr',
                                            start_value=1.0, end_value=0.0, cycle_size=10)
     _test(lr_scheduler, optimizer)