Repository navigation
material-train-native: expose libcvc#596's sampled L_multi starts; fix the native-parity docstrings - #115
Merged
Conversation
…x the native-parity docstrings libcvc#596 gave nav_material_trainer_create five trailing positional args after use_cuda: ms_count, frac_lo, frac_hi, seed and ms_semi_implicit. Their defaults are the legacy single explicit-Euler start. MaterialTrainer now takes them as ms_count=1, frac_range=(0.9, 0.9), seed=0 and semi_implicit=False. The seed keeps its low 32 bits, as torch's manual_seed does. Step k draws with seed + k, and loss() draws with the base seed. An older pycvc rejects the extra args with TypeError. hasattr cannot see new trailing args, so HAS_MATERIAL_TRAINER_MULTISTART probes for them: a create with an inverted frac range. The new binding range-checks before it opens the model, so it raises RuntimeError without a .cvcnm or a trainer. An old one raises TypeError on the argument count. The probe's types must match the C signature exactly, because SWIG's overload dispatch also answers a type mismatch with TypeError. A test cross-checks the probe against the proxy's Python signature. The legacy start keeps calling the old signature. That covers the defaults, and also any ms_count >= 1 or seed with a (0.9, 0.9) range and explicit Euler, since a degenerate range draws nothing and runs one rollout. So the material fork's ms_count=10 config still builds on an old pycvc. Any other configuration raises a RuntimeError naming libcvc#596 when the binding lacks the args. A bad frac_range raises ValueError on any binding, with multi_start_penalty's rule. The docstrings overclaimed native parity. surrogate_robust's multi_start_penalty said the native port "mirrors that single start", and the audit test said (0.9, 0.9) is libcvc's native port "exactly". The start matches, but native's default rollout is EXPLICIT Euler, the material fork's order that its train_material.py imports. GRL-SNAM's integrate_surrogate_v2 is semi-implicit. They match only with semi_implicit=True. The cvc-nav guide called the integrator split "not yet reconciled"; it now says how to close it. Tests: - test_material_train_native mirrors libcvc's test_sampled_multi_start_args. It checks that a degenerate range is the legacy loss, that a sampled range and the integrator switch change the loss, that seed + 2**32 matches seed, and that step k uses seed + k at lr=0. - A randomized cross-language parity test isolates native L_multi as loss(w_multi=1) - loss(w_multi=0) with semi_implicit=True. It compares that to multi_start_penalty, fed nav_matnet_forward's coefficients and a manual_seed generator. Native agrees to ~1e-7 relative; the explicit default is 5-30% off. These skip unless the pycvc carries the args. - test_material_train_native_args checks the signature routing against a fake binding, so it runs without pycvc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to transfix/libcvc#596, which ports #113's sampled multi-start to native
cvc::nav::multi_start_penaltyas an opt-in.What changes
MaterialTrainertakes the new create args (grl_snam/material_train_native.py)New kwargs:
ms_count=1,frac_range=(0.9, 0.9),seed=0,semi_implicit=False. They map tonav_material_trainer_create's new trailing positionals (ms_count, frac_lo, frac_hi, seed, ms_semi_implicit).seedare passed, astorch.Generator.manual_seedkeeps.seed + k;loss()uses the base seed.An older pycvc rejects the extra positionals with
TypeError, andhasattrcannot see new trailing args. So the new flagHAS_MATERIAL_TRAINER_MULTISTARTprobes for them with a create call that uses an inverted frac range:RuntimeErrorwithout a.cvcnmor a trainer.TypeErroron the argument count.The probe's argument types must match the C signature exactly, because SWIG's overload dispatch also answers a type mismatch with
TypeError. A test cross-checks the probe against the proxy's Python signature.Routing between the two signatures
ms_count >= 1orseedwith(0.9, 0.9)and explicit Euler: a degenerate range draws nothing and runs one rollout. The fork'sms_count=10config therefore still builds on an old pycvc.RuntimeErrornaming libcvc#596 when the binding lacks the args.frac_rangeraisesValueErroron any binding, withmulti_start_penalty's rule.Docstrings no longer overclaim native parity
surrogate_robust.multi_start_penaltysaid native "mirrors that single start".test_multi_start_degenerate_range_reproduces_the_legacy_single_startsaid(0.9, 0.9)is "(and libcvc's native port) exactly".The start matches, but the rollout does not. Native's default is EXPLICIT Euler, the material fork's order that its
train_material.pyimports. GRL-SNAM'sintegrate_surrogate_v2is semi-implicit. The two match only withsemi_implicit=True. The cvc-nav guide's "not yet reconciled" note now says how to close the gap.Tests
test_sampled_multi_start_argsmirrors libcvc'sbindings/pycvc/test_pycvc_nav_train.pythrough the wrapper:semi_implicitchange the loss;seed + 2**32gives the same loss asseed;seed + katlr=0;test_semi_implicit_l_multi_matches_the_torch_reference(3 randomized cases) is the cross-language parity check. Native L_multi is isolated asloss(w_multi=1) - loss(w_multi=0). The reference ismulti_start_penalty, fednav_matnet_forward's coefficients andtorch.Generator().manual_seed(seed). Over 8 local cases:seed + 1is ≥ 9e-4 off, so the tolerance also catches a wrong draw.Both of these skip unless the installed pycvc carries the new args.
tests/test_material_train_native_args.pychecks the probe, the signature routing, the arg types, seed masking and the error paths against a fake binding. It runs without pycvc.Validation
pycvc-cp312==3.5.1+cvc.2on this Windows box (linux-only). I used a local pycvc built from #596's head3c37c419, whose nav sources and binding are identical to the merge6fd5071f. The touched test files: 48 passed.use_cuda): 40 passed, 7 skipped, 1 failed. The failure istest_native_trainer_reduces_loss, which fails identically onmainbecause that pycvc predatesuse_cuda.main: run with the old dbg-deps pycvc: this branch had 19 failed, 477 passed, 18 skipped;mainhad 19 failed, 452 passed, 14 skipped. The FAILED lists are identical (the libm tripwire and native-parity tests against the stale local pycvc). The extra 25 passes and 4 skips are the new tests.CI expectations
The
publish-cvcpkgdispatch on libcvc master44f0fdde(which includes #596) publishedpycvc-cp312andpycvc-cp3133.5.1+cvc.2for linux-x86_64. The grl-snam recipes don't pin pycvc, so the cp312 and cp313 legs should run the new native tests. That run's cp311 linux-release cell failed and published nothing, so cp311 should resolve an older pycvc and skip them. No Windows pycvc was published.