Skip to content

Commit e2e14d2

Browse files
committed
addressed some MR comments
Signed-off-by: mikail <mkhona@nvidia.com>
1 parent 3acfc42 commit e2e14d2

2 files changed

Lines changed: 16 additions & 26 deletions

File tree

emerging_optimizers/scalar_optimizers/laprop.py

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ class LaProp(WeightDecayMixin, torch.optim.Optimizer):
6565
eps: Term added to the denominator for numerical stability.
6666
weight_decay: Weight decay coefficient.
6767
correct_bias: Whether to apply bias correction to the first and second moment EMAs.
68-
normalize: Whether to normalize each updated parameter back to its pre-update Frobenius norm.
69-
normalize_eps: Term used to avoid division by zero during parameter normalization.
68+
frob_normalize: Whether to normalize each updated parameter back to its pre-update Frobenius norm.
7069
weight_decay_method: Method to apply weight decay, see
7170
:class:`~emerging_optimizers.mixin.WeightDecayMixin` for more details.
7271
"""
@@ -79,8 +78,7 @@ def __init__(
7978
eps: float = 1e-8,
8079
weight_decay: float = 0.0,
8180
correct_bias: bool = True,
82-
normalize: bool = False,
83-
normalize_eps: float = 1e-8,
81+
frob_normalize: bool = False,
8482
weight_decay_method: WeightDecayT = "decoupled",
8583
) -> None:
8684
if not 0.0 <= lr:
@@ -91,16 +89,13 @@ def __init__(
9189
raise ValueError(f"Invalid beta at index 1: {betas[1]}")
9290
if not 0.0 <= eps:
9391
raise ValueError(f"Invalid epsilon value: {eps}")
94-
if not 0.0 < normalize_eps:
95-
raise ValueError(f"Invalid normalize_eps value: {normalize_eps}")
9692
if not 0.0 <= weight_decay:
9793
raise ValueError(f"Invalid weight_decay value: {weight_decay}")
98-
if normalize and weight_decay != 0.0:
99-
logging.warning("LaProp with normalize=True is intended to be used with weight_decay=0.0.")
94+
if frob_normalize and weight_decay != 0.0:
95+
logging.warning("LaProp with frob_normalize=True is intended to be used with weight_decay=0.0.")
10096
defaults = dict(lr=lr, betas=betas, eps=eps, weight_decay=weight_decay, correct_bias=correct_bias)
10197
self.weight_decay_method = weight_decay_method
102-
self.normalize = normalize
103-
self.normalize_eps = normalize_eps
98+
self.frob_normalize = frob_normalize
10499
super().__init__(params, defaults)
105100

106101
@torch.no_grad()
@@ -170,7 +165,7 @@ def step(self, closure: Callable[[], float] | None = None) -> float | None:
170165
grad = p.grad
171166
state = self.state[p]
172167
state["step"] += 1
173-
pre_norm = p.data.norm() if self.normalize else None
168+
pre_norm = p.data.norm() if self.frob_normalize else None
174169

175170
self._apply_weight_decay_inplace(p.data, grad, lr, weight_decay)
176171

@@ -184,7 +179,8 @@ def step(self, closure: Callable[[], float] | None = None) -> float | None:
184179
eps,
185180
)
186181
p.data.add_(update, alpha=-lr)
187-
if pre_norm is not None:
188-
p.data.mul_(pre_norm / p.data.norm().clamp_min(self.normalize_eps))
182+
if self.frob_normalize:
183+
assert pre_norm is not None
184+
p.data.mul_(pre_norm / p.data.norm().clamp_min(eps))
189185

190186
return loss

tests/test_laprop.py

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ def test_state_initialization(self, shape) -> None:
7171
expected_exp_avg_sq = (1 - beta2) * grad.square()
7272
normalized_grad = grad / (grad.abs() + optimizer.param_groups[0]["eps"])
7373
expected_exp_avg = (1 - beta1) * normalized_grad
74-
torch.testing.assert_close(optimizer.state[param]["exp_avg_sq"], expected_exp_avg_sq)
75-
torch.testing.assert_close(optimizer.state[param]["exp_avg"], expected_exp_avg)
74+
torch.testing.assert_close(optimizer.state[param]["exp_avg_sq"], expected_exp_avg_sq, atol=0, rtol=0)
75+
torch.testing.assert_close(optimizer.state[param]["exp_avg"], expected_exp_avg, atol=0, rtol=0)
7676

7777
@parameterized.parameters(
7878
{"shape": (3, 3)},
@@ -96,9 +96,9 @@ def test_optimizer_step_matches_update_function(self, shape) -> None:
9696
param.grad = grad.clone()
9797
optimizer.step()
9898

99-
torch.testing.assert_close(param, old_param - lr * expected_update)
100-
torch.testing.assert_close(optimizer.state[param]["exp_avg"], exp_avg)
101-
torch.testing.assert_close(optimizer.state[param]["exp_avg_sq"], exp_avg_sq)
99+
torch.testing.assert_close(param, old_param - lr * expected_update, atol=0, rtol=0)
100+
torch.testing.assert_close(optimizer.state[param]["exp_avg"], exp_avg, atol=0, rtol=0)
101+
torch.testing.assert_close(optimizer.state[param]["exp_avg_sq"], exp_avg_sq, atol=0, rtol=0)
102102

103103
@parameterized.parameters(
104104
{"shape": (3, 3)},
@@ -118,10 +118,10 @@ def test_no_grad_no_update_params_unchanged(self, shape) -> None:
118118
{"shape": (15, 31)},
119119
{"shape": (127, 255)},
120120
)
121-
def test_normalize_preserves_parameter_norm(self, shape) -> None:
121+
def test_frob_normalize_preserves_parameter_norm(self, shape) -> None:
122122
"""LaProp can normalize updated parameters back to their pre-update norm."""
123123
param = torch.nn.Parameter(torch.randint(1, 5, shape, device=self.device, dtype=torch.float32))
124-
optimizer = LaProp([param], lr=0.25, weight_decay=0.0, normalize=True)
124+
optimizer = LaProp([param], lr=0.25, weight_decay=0.0, frob_normalize=True)
125125
param.grad = torch.randint(1, 5, shape, device=self.device, dtype=torch.float32)
126126
original_norm = param.norm()
127127

@@ -168,12 +168,6 @@ def test_negative_eps_raises_value_error(self) -> None:
168168
with self.assertRaisesRegex(ValueError, "Invalid epsilon"):
169169
LaProp([param], eps=-1e-8)
170170

171-
def test_non_positive_normalize_eps_raises_value_error(self) -> None:
172-
"""Test that LaProp raises ValueError for non-positive normalize_eps."""
173-
param = torch.nn.Parameter(torch.randn(3, 3, device=self.device))
174-
with self.assertRaisesRegex(ValueError, "Invalid normalize_eps"):
175-
LaProp([param], normalize_eps=0.0)
176-
177171
def test_negative_weight_decay_raises_value_error(self) -> None:
178172
"""Test that LaProp raises ValueError for negative weight_decay."""
179173
param = torch.nn.Parameter(torch.randn(3, 3, device=self.device))

0 commit comments

Comments
 (0)