primus data diffusion-encoded: CLI flags are silently ignored, and config keys silently vanish, whenever a value equals its parser default #1001

Summary

primus data diffusion-encoded merges a YAML config with CLI flags. The documented contract is that CLI flags win — stated in the docstring of _load_config_with_cli_overrides ("Priority order: 1. Explicitly provided CLI arguments, 2. YAML config values") and printed at runtime as Configuration merged (CLI args override YAML).

That contract is broken. The merge has no way of knowing which flags were actually typed, so it infers it by comparing each parsed value against a hardcoded default table. That single comparison causes two distinct failures:

• A (silent, worse): a flag you typed is discarded if its value happens to equal the parser default, and the YAML value is used instead. No error, no warning.
• B (crash): a setting present in neither the YAML nor the typed flags is dropped from the namespace entirely instead of falling back to its default, causing AttributeError downstream.

Both are reproducible from a clean clone in about a minute, and neither requires a GPU — this is pure configuration handling.

Root cause

primus/cli/subcommands/data.py, in _load_config_with_cli_overrides:

# line 379 — the result is seeded from the YAML only
merged_dict = flat_config.copy()

# lines 385-395 — a CLI value is kept only if it differs from the default
for key, cli_value in cli_args.items():
    if key == "config":
        merged_dict["config"] = cli_value
        continue
    default_value = parser_defaults.get(key)
    if cli_value != default_value:      # <-- line 393, the defect
        merged_dict[key] = cli_value

# line 398 — a brand-new namespace, built from that dict alone
return argparse.Namespace(**merged_dict)

argparse fills untyped options with their defaults, so "value equals default" and "user did not type this flag" are indistinguishable in the parsed namespace. Line 393 treats them as the same thing. They are not:

• A typed flag whose value equals the default is misread as untyped, so it loses to the YAML. (Symptom A)
• Because line 398 discards the original namespace and rebuilds from merged_dict, any key filtered out by line 393 and not supplied by the YAML ceases to exist. (Symptom B)

Symptom B is aggravated by two things. First, _flatten_preprocessing_config is gated per YAML section (lines 289-294), so omitting the optional image: block means image_size / variable_size / center_crop / max_size never enter flat_config at all. Second, the complete default table at lines 311-341 does contain "image_size": 1024 — but it is only ever read as the right-hand side of that !=, never used as a source of values. The defaults exist in two places in this file and are applied in neither.

Worth noting: almost every other read in this file is already defensive — getattr(args, ..., default) at lines 138, 167-172, 496, 505-510. Only lines 489-492 use bare args.x. The surrounding code has been hardened against this exact hole one getattr at a time, rather than the merge being fixed once.

Why the existing tests could not catch this

TestLoadConfigWithCliOverrides in tests/unit_tests/cli/test_data_config.py hand-builds the namespace:

args = argparse.Namespace(config="test.yaml", **{k: v for k, v in defaults.items() if k != "config"})

Every key is populated, so the fixture inherits the same ambiguity as the bug: there is no way to express "flag omitted" versus "flag typed with a value equal to the default", and the broken case is precisely the one that cannot be written down. Any fix has to change this calling convention — presence in the namespace must mean "explicitly provided" — so these tests need to drive the real parser instead of constructing a namespace by hand.

Reproduction

No GPU, no ROCm, no torch install needed. Run inside any rocm/primus image so the module's torch-dependent imports resolve. The shallow clone below is deliberate: no submodules, requirements, or accelerator are required, because the defect is entirely in argument parsing.

1. Clone

git clone --depth 1 https://github.com/AMD-AGI/Primus.git && cd Primus
git log -1 --oneline    # b8d3cc5

2. cfg_a.yaml — every value deliberately not the parser default, so YAML and CLI genuinely disagree

source:
  type: huggingface
  hf_dataset: diffusers/pokemon-gpt4-captions
output:
  output_dir: /tmp/out
model:
  batch_size: 64        # parser default is 8
  precision: fp32       # parser default is bf16
  t5_max_length: 256    # parser default is 512
image:
  image_size: 512

3. cfg_b.yaml — a valid config that simply omits the optional image: section

source:
  type: huggingface
  hf_dataset: diffusers/pokemon-gpt4-captions
output:
  output_dir: /tmp/out
model:
  model_path: black-forest-labs/FLUX.1-dev
  batch_size: 16

4. bug1.py — drives the real merge function through the real parser

import argparse
from primus.cli.subcommands import data

parser = argparse.ArgumentParser(prog="primus")
data.register_subcommand(parser.add_subparsers(dest="command"))

def merged(argv):
    return data._load_config_with_cli_overrides(parser.parse_args(argv))

print("=== Symptom A: flags whose value equals the parser default ===")
a = merged(["data", "diffusion-encoded", "--config", "cfg_a.yaml",
            "--batch-size", "8", "--precision", "bf16", "--t5-max-length", "512"])
print("  typed --batch-size 8        -> batch_size    =", a.batch_size)
print("  typed --precision bf16      -> precision     =", a.precision)
print("  typed --t5-max-length 512   -> t5_max_length =", a.t5_max_length)

print("=== control: same flag, a non-default value ===")
c = merged(["data", "diffusion-encoded", "--config", "cfg_a.yaml", "--batch-size", "16"])
print("  typed --batch-size 16       -> batch_size    =", c.batch_size)

print("=== Symptom B: config with no image: section ===")
b = merged(["data", "diffusion-encoded", "--config", "cfg_b.yaml"])
data._validate_preprocessing_config(b)
print("  _validate_preprocessing_config() passed")
print("  hasattr(args, 'image_size') =", hasattr(b, "image_size"))
print("  args.image_size =", b.image_size)

5. Run

docker run --rm -v "$PWD:/work" -w /work rocm/primus:v26.5 \
  python3 -u bug1.py 2>&1 | grep -v 'already registered'

Actual output

=== Symptom A: flags whose value equals the parser default ===
  typed --batch-size 8        -> batch_size    = 64
  typed --precision bf16      -> precision     = fp32
  typed --t5-max-length 512   -> t5_max_length = 256
=== control: same flag, a non-default value ===
  typed --batch-size 16       -> batch_size    = 16
=== Symptom B: config with no image: section ===
  _validate_preprocessing_config() passed
  hasattr(args, 'image_size') = False
Traceback (most recent call last):
  File "/work/bug1.py", line 27, in <module>
    print("  args.image_size =", b.image_size)
AttributeError: 'Namespace' object has no attribute 'image_size'

Expected vs actual

The last row is the control, and it isolates the mechanism. The same flag, the same config, the same command — obeyed at 16 and discarded at 8, for no reason other than 8 being the parser default. It also rules out the mundane explanations (wrong config path, misspelled flag, intentionally inverted precedence), all of which would fail at 16 too.

Note also that typing the default is not a no-op here: with cfg_a.yaml loaded the effective batch size is 64, so --batch-size 8 is a meaningful instruction that the tool drops.

For Symptom B, image_size should resolve to its default of 1024 — nobody overrode it — but instead the key is absent from the namespace, after validation has already declared the config valid.

Symptom B, end-to-end with no test code

docker run --rm -v "$PWD:/work" -w /work rocm/primus:v26.5 \
  python3 -m primus.cli.main data diffusion-encoded --config cfg_b.yaml 2>&1 \
  | grep -v 'already registered'

Loading config from: cfg_b.yaml
Configuration merged (CLI args override YAML)
...
  File "/work/primus/cli/subcommands/data.py", line 489, in _prepare_encoded
    variable_size=args.variable_size,
AttributeError: 'Namespace' object has no attribute 'variable_size'
[Primus] Error: AttributeError: 'Namespace' object has no attribute 'variable_size' (primus/cli/subcommands/data.py:489)

An ordinary user command with an ordinary valid config, and the tool prints the guarantee it is about to break two lines before breaking it.

What this can affect

diffusion-encoded is an offline preprocessing step: it runs the VAE over images and T5/CLIP over captions and bakes the resulting tensors into an Energon WebDataset. The settings this bug corrupts are precisely the ones that determine what gets baked in — image_size and the crop options set the geometry, precision sets the numeric type, t5_max_length sets where captions are truncated.

Because the artifact is written once and trained on repeatedly, Symptom A does not produce a slow run or a visible error. It produces a dataset that looks fine, and the failure surfaces much later as unexplained training-quality loss with nothing pointing back at the preprocessing command. The only remedy is re-encoding the whole dataset.

A concrete, entirely plausible instance: the help text for --t5-max-length reads "default: 512, use 256 for FLUX.1-schnell". So a team keeps a shared schnell config pinned at 256, and someone doing a FLUX.1-dev run passes --t5-max-length 512 — the documented way to do it. The flag is silently discarded, and captions get encoded truncated at 256.

Scope of the affected surface:

• Symptom A applies to the options whose default is a real value: hf_split, shard_size, model_path, precision, device, batch_size, t5_max_length, image_size, max_size, vae_latent_mode.
• Options defaulting to None (vae_path, t5_path, clip_path, max_samples, hf_data_files, hf_token_file, source_type, output_dir, input_dir, input_path, hf_dataset) are immune to Symptom A, because None cannot be typed on a command line. The store_true / store_false flags (--compress, --variable-size, --center-crop) are immune for the same structural reason: a flag's only typeable value is the opposite of its default, so it always registers as differing. Both verified against the pre-fix code.
• Symptom B applies to any key whose YAML section is absent from the file, since _flatten_preprocessing_config is gated per section. The image:, data_format:, and auth: sections are all optional. Only the four image: keys crash today, because the rest of the reads are already wrapped in getattr.
• Only diffusion-encoded is affected. diffusion-raw shares _add_common_args but has no --config and never calls the merge, so its defaults arrive intact — including at lines 150-153, where it reads the same four keys with bare attribute access.

Test environment

Reproduced on a fresh clone at commit b8d3cc52c83a64837bbb12357b72362e1d519354 (b8d3cc5, 2026-08-20).

The GPU details are recorded for completeness only — the bug is in argument parsing and reproduces with no accelerator involved. The container is used solely to satisfy the module's torch-dependent imports.