perf(config): change default max_sequences_per_example to 10 when dp is off (#78)

<!-- Thank you for contributing to Safe Synthesizer! -->

# Summary
Change the default max_sequences_per_example to 10 when dp is off. This
is from our experiment findings that this gives good performance and
decent runtime.

## Pre-Review Checklist

<!-- These checks should be completed before a PR is reviewed, -->
<!-- but you can submit a draft early to indicate that the issue is
being worked on. -->

Ensure that the following pass:

- [x] `make format && make lint` or via prek validation.
- [x] `make test` passes locally
- [ ] `make e2e` passes locally
- [ ] `make test-ci-container` passes locally (recommended)

## Pre-Merge Checklist

<!-- These checks need to be completed before a PR is merged, -->
<!-- but as PRs often change significantly during review, -->
<!-- it's OK for them to be incomplete when review is first requested.
-->

- [x] New or updated tests for any fix or new behavior
- [x] Updated documentation for new features and behaviors, including
docstrings for API docs.

## Other Notes

<!-- Please add the issue number that should be closed when this PR is
merged. -->
- Closes #73

---------

Signed-off-by: Nina Xu <ninaxu@cs-oci-ord-vscode-01.cm.cluster>
Co-authored-by: Nina Xu <ninaxu@cs-oci-ord-vscode-01.cm.cluster>

Author: nina-xu

src/nemo_safe_synthesizer/config/autoconfig.py

@@ -11,7 +11,7 @@
 import math
 import re
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Any, Callable, Literal
+from typing import TYPE_CHECKING, Any, Callable
 
 import pandas as pd
 from pydantic import GetCoreSchemaHandler
@@ -228,24 +228,39 @@ def _determine_delta(self) -> dict[str, float]:
         )
         return {"delta": d}
 
-    def _determine_max_sequences_per_example(self) -> dict[str, Literal["auto"] | int | None]:
+    def _determine_max_sequences_per_example(self) -> dict[str, int | None]:
         """
         Determine max_sequences_per_example if set to auto.
 
         Returns:
-            Dict with max_sequences_per_example if auto-determined, empty dict otherwise.
+            Dict with max_sequences_per_example resolved to a concrete value:
+            1 if DP is enabled, 10 if auto with DP disabled, or the
+            explicit value (int) if manually specified, or None if not specified.
         """
         if self._dp_enabled is True:
-            logger.info(
-                "Parameter `max_sequences_per_example` was automatically set "
-                "to 1 based on the use of differential privacy."
-            )
+            if self._config.data.max_sequences_per_example in [None, AUTO_STR, 1]:
+                logger.info(
+                    "Parameter `max_sequences_per_example` was automatically set "
+                    "to 1 based on the use of differential privacy."
+                )
+            else:
+                logger.info(
+                    "Parameter `max_sequences_per_example` does not allow the value of "
+                    "{self._config.data.max_sequences_per_example} when DP is enabled. Setting to 1 instead."
+                )
             return {"max_sequences_per_example": 1}
         elif self._config.data.max_sequences_per_example != AUTO_STR:
+            if self._config.data.max_sequences_per_example is None:
+                logger.info(
+                    "Parameter `max_sequences_per_example` is not specified, so each example will fill up the context window."
+                )
             return {"max_sequences_per_example": self._config.data.max_sequences_per_example}
 
         else:
-            return {"max_sequences_per_example": None}
+            logger.info(
+                "Parameter `max_sequences_per_example` was automatically set to 10 for best performance/efficiency."
+            )
+            return {"max_sequences_per_example": 10}
 
     def _build_updated_params(
         self,
@@ -303,7 +318,7 @@ def resolve(self) -> SafeSynthesizerParameters:
 class AutoParamsValidator:
     value_func: Callable[[Any], bool]
 
-    def validate(self, value):
+    def validate(self, value, _info):
         if isinstance(value, str) and value == "auto":
             return value
         elif self.value_func(value):

src/nemo_safe_synthesizer/config/data.py

@@ -76,10 +76,10 @@ class DataParameters(Parameters):
         OptionalAutoInt,
         Field(
             description=(
-                "If specified, adds at most this number of sequences per example; "
-                "otherwise, fills up context. Supports 'auto' where a value of 1 is "
-                "chosen if differential privacy is enabled, and None otherwise. "
-                "Required for DP to limit contribution of each example."
+                "If specified, adds at most this number of sequences per example. "
+                "Supports 'auto' where a value of 1 is chosen if differential privacy is "
+                "enabled, and 10 otherwise. If not specified or set to 'auto', fills up "
+                "context. Required for DP to limit contribution of each example."
             ),
         ),
     ] = AUTO_STR

src/nemo_safe_synthesizer/config/parameters.py

@@ -88,8 +88,8 @@ def check_dp_compatibility(
 
         if not dp_params.dp_enabled:
             if data.max_sequences_per_example is not None and data.max_sequences_per_example == AUTO_STR:
-                logger.debug("setting max_sequences_per_example to None because DP is disabled")
-                data.max_sequences_per_example = None
+                logger.debug("setting max_sequences_per_example to the default of 10 because DP is disabled")
+                data.max_sequences_per_example = 10
             return dp_params
 
         match data.max_sequences_per_example:

tests/config/test_autoconfig.py

@@ -109,7 +109,7 @@ def get_config(self) -> SafeSynthesizerParameters:
         num_input_records_to_sample=None,  # Will be auto-resolved
         delta=None,  # Not used (DP disabled)
         dp_enabled=False,
-        max_seq=None,  # "auto" with no DP -> None
+        max_seq=10,  # "auto" with no DP -> 10
     ),
 )
 
@@ -134,27 +134,6 @@ def get_config(self) -> SafeSynthesizerParameters:
     ),
 )
 
-AUTO_WITH_DP_NULL_MAX_SEQ = AutoConfigTestCase(
-    name="auto_with_dp_null_max_seq",
-    config=SafeSynthesizerParameters(
-        training=TrainingHyperparams(
-            rope_scaling_factor="auto",
-            num_input_records_to_sample="auto",
-            use_unsloth="auto",
-        ),
-        data=DataParameters(max_sequences_per_example=None),  # Explicit None
-        privacy=DifferentialPrivacyHyperparams(dp_enabled=True, delta="auto"),
-    ),
-    expected=Expected(
-        use_unsloth=False,  # "auto" resolves to False when DP enabled
-        rope_scaling_factor=None,  # Will be auto-resolved to an int
-        num_input_records_to_sample=None,  # Will be auto-resolved
-        delta=None,  # Will be auto-resolved based on data size
-        dp_enabled=True,
-        max_seq=1,  # DP enabled -> always 1 (even with None input)
-    ),
-)
-
 EXPLICIT = AutoConfigTestCase(
     name="explicit",
     config=SafeSynthesizerParameters(
@@ -205,7 +184,6 @@ def get_config(self) -> SafeSynthesizerParameters:
 ALL_TEST_CASES: list[AutoConfigTestCase] = [
     AUTO_NO_DP,
     AUTO_WITH_DP,
-    AUTO_WITH_DP_NULL_MAX_SEQ,
     EXPLICIT,
     DP_WITH_UNSLOTH_TRUE,
 ]
@@ -321,15 +299,23 @@ def test_determine_delta(self, data_size, test_case, config, expected):
         else:
             assert result == {}
 
-    def test_determine_max_sequences_per_example(self, sample_data, config, expected):
-        """Max sequences should be 1 for DP, None for non-DP auto, or explicit value."""
-        resolver = AutoConfigResolver(sample_data, config)
-
-        # Verify validation already resolved the config value
-        assert config.data.max_sequences_per_example == expected.max_seq
-
+    @pytest.mark.parametrize(
+        "max_seq_input, expected_max_seq",
+        [
+            pytest.param("auto", [1, 10], id="auto_max_seq"),  # dp_enabled=True -> 1, dp_enabled=False -> 10
+            pytest.param(5, [1, 5], id="explicit_max_seq"),  # dp_enabled=True -> 1, dp_enabled=False -> 5
+            pytest.param(None, [1, None], id="none_max_seq"),  # dp_enabled=True -> 1, dp_enabled=False -> None
+        ],
+    )
+    def test_determine_max_sequences_per_example(self, sample_data, config, max_seq_input, expected_max_seq):
+        """Max sequences should be 1 for DP regardless of input; for non-DP, auto -> 10, explicit -> explicit value, None -> None."""
+        config_copy = config.model_copy(deep=True)
+        config_copy.data.max_sequences_per_example = max_seq_input
+        resolver = AutoConfigResolver(sample_data, config_copy)
         result = resolver._determine_max_sequences_per_example()
-        assert result == {"max_sequences_per_example": expected.max_seq}
+
+        expected_index = 0 if config_copy.privacy.dp_enabled else 1
+        assert result == {"max_sequences_per_example": expected_max_seq[expected_index]}
 
     def test_resolve(self, sample_data, config, expected):
         """Full resolution should produce valid SafeSynthesizerParameters.