☑️💸 Update Losses' HPO default range checks (#1337)

* [x] check instantiation of losses for a few configurations derived
from the `hpo_default`
* [x] fix an issue with the default values of `DoubleMarginLoss`
([bb88719](bb88719))
* [x] add suitable `hpo_default`s to the general
  - `MarginPairwiseLoss`
  - `DeltaPointwiseLoss`

Fix #1334

Author: mberr

src/pykeen/losses.py

@@ -455,10 +455,18 @@ class MarginPairwiseLoss(PairwiseLoss):
     function like the ReLU or softmax, and $\lambda$ is the margin.
     """
 
+    hpo_default: ClassVar[Mapping[str, Any]] = dict(
+        margin=DEFAULT_MARGIN_HPO_STRATEGY,
+        margin_activation=dict(
+            type="categorical",
+            choices=margin_activation_resolver.options,
+        ),
+    )
+
     def __init__(
         self,
-        margin: float,
-        margin_activation: Hint[nn.Module],
+        margin: float = 1.0,
+        margin_activation: Hint[nn.Module] = None,
         reduction: str = "mean",
     ):
         r"""Initialize the margin loss instance.
@@ -730,6 +738,10 @@ def resolve_margin(
         :raises ValueError:
             In case of an invalid combination.
         """
+        # 0. default
+        if all(p is None for p in (positive_margin, negative_margin, offset)):
+            return 1.0, 0.0
+
         # 1. positive & negative margin
         if positive_margin is not None and negative_margin is not None and offset is None:
             if negative_margin > positive_margin:
@@ -771,8 +783,8 @@ def resolve_margin(
     def __init__(
         self,
         *,
-        positive_margin: Optional[float] = 1.0,
-        negative_margin: Optional[float] = 0.0,
+        positive_margin: Optional[float] = None,
+        negative_margin: Optional[float] = None,
         offset: Optional[float] = None,
         positive_negative_balance: float = 0.5,
         margin_activation: Hint[nn.Module] = "relu",
@@ -905,6 +917,14 @@ class DeltaPointwiseLoss(PointwiseLoss):
     =============================  ==========  ======================  ========================================================  =============================================
     """  # noqa:E501
 
+    hpo_default: ClassVar[Mapping[str, Any]] = dict(
+        margin=DEFAULT_MARGIN_HPO_STRATEGY,
+        margin_activation=dict(
+            type="categorical",
+            choices=margin_activation_resolver.options,
+        ),
+    )
+
     def __init__(
         self,
         margin: Optional[float] = 0.0,

tests/cases.py

@@ -2,6 +2,7 @@
 
 """Test cases for PyKEEN."""
 import inspect
+import itertools
 import logging
 import os
 import pathlib
@@ -61,7 +62,7 @@
 from pykeen.datasets.mocks import create_inductive_dataset
 from pykeen.datasets.nations import NATIONS_TEST_PATH, NATIONS_TRAIN_PATH
 from pykeen.evaluation import Evaluator, MetricResults, evaluator_resolver
-from pykeen.losses import Loss, PairwiseLoss, PointwiseLoss, SetwiseLoss, UnsupportedLabelSmoothingError
+from pykeen.losses import Loss, PairwiseLoss, PointwiseLoss, SetwiseLoss, UnsupportedLabelSmoothingError, loss_resolver
 from pykeen.metrics import rank_based_metric_resolver
 from pykeen.metrics.ranking import (
     DerivedRankBasedMetric,
@@ -241,6 +242,27 @@ def tearDown(self) -> None:
         self.directory.cleanup()
 
 
+def iter_from_space(key: str, space: Mapping[str, Any]) -> Iterable[MutableMapping[str, Any]]:
+    """Iterate over some configurations for a single hyperparameter."""
+    typ = space["type"]
+    if typ == "categorical":
+        for choice in space["choices"]:
+            yield {key: choice}
+    elif typ in (float, int):
+        yield {key: space["low"]}
+        yield {key: space["high"]}
+    else:
+        raise NotImplementedError(typ)
+
+
+def iter_hpo_configs(hpo_default: Mapping[str, Mapping[str, Any]]) -> Iterable[Mapping[str, Any]]:
+    """Iterate over some representative configurations from the HPO default."""
+    for combination in itertools.product(
+        *(iter_from_space(key=key, space=space) for key, space in hpo_default.items())
+    ):
+        yield ChainMap(*combination)
+
+
 class LossTestCase(GenericTestCase[Loss]):
     """Base unittest for loss functions."""
 
@@ -354,8 +376,21 @@ def test_optimization_direction_slcwa(self):
     def test_hpo_defaults(self):
         """Test hpo defaults."""
         signature = inspect.signature(self.cls.__init__)
+        # check for invalid keys
         invalid_keys = set(self.cls.hpo_default.keys()).difference(signature.parameters)
         assert not invalid_keys
+        # check that each parameter without a default occurs
+        required_parameters = {
+            parameter_name
+            for parameter_name, parameter in signature.parameters.items()
+            if parameter.default == inspect.Parameter.empty
+            and parameter.kind not in {inspect.Parameter.VAR_KEYWORD, inspect.Parameter.VAR_POSITIONAL}
+        }.difference({"self"})
+        missing_required = required_parameters.difference(self.cls.hpo_default.keys())
+        assert not missing_required
+        # try to instantiate loss for some configurations in the HPO search space
+        for config in iter_hpo_configs(self.cls.hpo_default):
+            assert loss_resolver.make(query=self.cls, pos_kwargs=config)
 
 
 class PointwiseLossTestCase(LossTestCase):

tests/test_losses.py

@@ -149,6 +149,18 @@ class SoftMarginrankingLossTestCase(cases.GMRLTestCase):
     cls = pykeen.losses.SoftMarginRankingLoss
 
 
+class MarginPairwiseLossTestCase(cases.GMRLTestCase):
+    """Tests for general margin pairwise loss."""
+
+    cls = pykeen.losses.MarginPairwiseLoss
+
+
+class DeltaPointwiseLossTestCase(cases.PointwiseLossTestCase):
+    """Tests for general delta point-wise loss."""
+
+    cls = pykeen.losses.DeltaPointwiseLoss
+
+
 class PairwiseLogisticLossTestCase(cases.GMRLTestCase):
     """Tests for the pairwise logistic loss."""
 
@@ -171,8 +183,6 @@ class TestLosses(unittest_templates.MetaTestCase[Loss]):
         pykeen.losses.PairwiseLoss,
         pykeen.losses.PointwiseLoss,
         pykeen.losses.SetwiseLoss,
-        pykeen.losses.DeltaPointwiseLoss,
-        pykeen.losses.MarginPairwiseLoss,
         pykeen.losses.AdversarialLoss,
     }