[vivekkhandelwal1]

A training run saves its LoRA only as a raw torch.save checkpoint. A score run, tools/merge_base_adapter.py and peft all take a PEFT adapter directory, so there was no way to use a trained model outside its run. This adds the missing step.

tools/export_adapter.py --ckpt RUN_DIR/ckpt/iter-<n> --out DIR:

• checks the checkpoint against its MANIFEST.json;
• writes adapter_model.safetensors (the fp32 tensors, same bits) and adapter_config.json (the trainer's r 16 / alpha 32, no dropout, no bias);
• reads the output back bit for bit, and records the input and output hashes in export-manifest.json.

A failed read-back stays in DIR.partial. The README's "Run outputs" section shows export, then score or merge.

The test (heavy, CPU) trains one wired iteration, exports the checkpoint, and merges it with the existing merge tool. The merged export must be exactly, bit for bit, the weights the engines were served, and peft must load the checkpoint's tensors. A tampered checkpoint or an existing output is refused. Two mutants (a scaled B, a skipped manifest check) are caught.

Limit: a run with sync.stochastic_rounding served stochastically rounded weights, so a nearest-rounded merge is close to them, not equal. On MI350X: I exported a real checkpoint from a 3-iteration training run of the HIP-kernel example (#613) and scored it with model.adapter_dir. The score run merged it, served it, and the landing check was PROVEN on 4 engines. That run also found the safetensors file written owner-only (0600) beside a 0644 config; it now takes the umask's mode.

[Copilot]

🟡 Changes recommended

The exporter can overwrite a concurrently created output directory, and reopening trainer.pt across verification, loading, and hashing can export unverified tensors or record a mismatched input digest.

2 open issues

• Use one immutable trainer buffer for verification, loading, and hashing · New
• Publish output without replacing a concurrently created destination · New

What changed in this PR

Adds PEFT adapter export for aorlta training checkpoints, allowing trained LoRA weights to be scored, merged, or loaded outside their original run.

Changes:

• Adds checkpoint verification, adapter serialization, bitwise read-back checks, and input/output hash recording.
• Adds CPU integration tests for merge equivalence, PEFT loading, and refusal of tampered checkpoints or existing outputs.
• Documents the export workflow and its limitations.

File	Description
aorlta/​tools/​export_adapter.py	Implements checkpoint-to-PEFT export and integrity checks.
aorlta/​tests/​tools/​test_export_adapter.py	Tests export compatibility and refusal cases.
aorlta/​README.md	Documents exporting, scoring, and merging trained adapters.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The export publication step can replace a concurrently created output directory, violating the non-overwrite guarantee instead of refusing the collision and preserving the partial export.

2 open issues

• Publish output without replacing a concurrently created destination
• Use one immutable trainer buffer for verification, loading, and hashing

1 previously missed issue

In code that hasn't changed since last review

Limit merged-weight equality claims to the tested CPU fixture

aorlta/​README.md:256

A GPU-trained checkpoint is not guaranteed to reproduce the served bits: training merges on the base tensors' device, but merge_base_adapter.py merges on CPU. Different fp32 accumulation orders can change the final bf16 rounding even without stochastic rounding. Please limit the merged-weight equality claim to the tested CPU fixture and distinguish it from bitwise preservation of the adapter tensors.

_{Review effort: Balanced}

[amd-vivekag]

Review summary

Verdict: fix-before-ship

The export shape is right: manifest check, fp32 bits unchanged, read-back, and a failed read-back left in .partial. The manifest can still certify a different file from the one that was loaded.

Findings: 0 High · 1 Medium · 0 Low

Copilot's other note, that rename replaces an empty directory created after the existence check, is a real POSIX race. It does not publish a passing export of the wrong tensors; the three reads of trainer.pt can. I am not asking to block on the rename.

[amd-vivekag]

[Medium] trainer.pt is read three times, and the manifest hashes a different read from the one that was loaded.

verify_checkpoint hashes the file, torch.load reads it again, and line 139 hashes it a third time. A replace between those reads exports tensors the manifest check never saw, with pass: true and a sha256 of some other bytes.

Fix: read the file once, check that digest against the checkpoint manifest, torch.load those bytes, and record that same digest. Copilot flagged this too.

[vivekkhandelwal1]

Fixed in 7258101 as you suggested: trainer.pt is read once, those bytes must match the checkpoint manifest's entry, they're loaded with torch.load(BytesIO(...)), and the same digest goes into the export manifest. A test replaces the file before, together with its manifest, and after the check.

[amd-vivekag]

Resolved — confirmed on the current head.

[Copilot]

🔵 Needs a closer look

Human judgment is needed to reconcile the documentation’s bitwise-reproducibility claims and CPU-only limitations with cross-device rounding differences and reported GPU validation.

No open issues 🎉

2 resolved since last review

• Publish output without replacing a concurrently created destination
• Use one immutable trainer buffer for verification, loading, and hashing

_{Review effort: Balanced}

[vivekkhandelwal1]

@amd-vivekag your point is addressed in 7258101: trainer.pt is read once, checked against the manifest, loaded from those bytes, and that digest recorded. Could you take another look?

[amd-vivekag]

No review comments — approving. The earlier findings are fixed on this head.

[Copilot]

🟡 Changes recommended

The README’s merged-weight equality claim must be limited to the tested CPU setup with matching thread counts, since other device or thread configurations can alter final bf16 rounding.

1 open issue

• Limit bitwise equality claims to matching arithmetic configurations · New

_{Review effort: Balanced}