Repository navigation
test: load every examples/ model once for the corpus sweeps - #2171
Open
devin-ai-integration[bot] wants to merge 5 commits into
Open
devin-ai-integration[bot] wants to merge 5 commits into
devin-ai-integration[bot] wants to merge 5 commits into
Conversation
TestExamplesSweep (tests/corpus) parses and exports each model under examples/ once and checks the Turtle and api-json round-trip ratchets, settled collections, the gRPC element-properties agreement and modelsync's without-derived read-back on that load, replacing five tests in four packages. The shards corpus-roundtrip, corpus-apijson and export-settled become corpus-sweep; scripts/race-shard.sh --sweeps prints the shards whose tests walk a downloaded corpus. The round-trip expectation files are re-recorded with their recorders: only the regenerate command in their headers changed. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…nnot stand in for it Co-Authored-By: jason.han <hanhuijun@gmail.com>
… corpus is absent Only the two round-trip ratchets stand down without every corpus; settled collections, the gRPC agreement and the modelsync read-back still cover the committed models and fixtures. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
There was a problem hiding this comment.
Devin Review found 2 findings. Devin automatically fixed all 2 findings.
Fixed by Devin (2)
- Absent corpora skip unrelated checks — corpusRoundTripFiles now reports absent roots instead of skipping; only the two round-trip ratchets stand down (and -update-* refuses), while settled collections, the gRPC agreement and the read-back still run over the committed models and fixtures. Verified with examples/sysml-v2-training moved aside: 800 models checked, ratchets announced as not run.
- Missing RDF summary passes CI gate — Both pr.yml and .circleci/config.yml now grep ': corpus round trip: N files:' (the go test file:line prefix precedes the RDF summary), which the api-json line cannot match.
Co-Authored-By: jason.han <hanhuijun@gmail.com>
This branch has not been deployed
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.
What and why
Every full-tier run spent a third of its test time walking
examples/(920 models) once per test file, each re-parsing and re-exporting every model.Every model under
examples/was parsed and exported once per test by five tests in four packages.TestExamplesSweep(tests/corpus) now loads each model once — read, parse,export.ToRDFWith, written as Turtle and api-json — and checks every per-model property on that load:testdata/corpus_roundtrip_expected.txt,-update-corpus-roundtrip) and the api-json one (testdata/api_json_roundtrip_expected.txt,-update-api-json-roundtrip): hop 1 is the shared writing, the hop back and hop 2 are the verdict's own as before;export.ModelToRDFWith, the model path, whose Turtle/api-json differ fromToRDFWith's on every example, so it is the one export the sweep does twice per.sysmlfile (parse shared);ParseFile, which is the property (the service's answer against the export). Each worker owns agrpc.Servicebecause the service's model cache is an LRU of four and a shared one could evict a live model between a worker's RPCs;Every property is asserted for exactly the models it was before (settled:
.sysmlthat parse clean, examples + export fixtures; agreement: examples outsidepilot-corpora/; read-back:examples/*.sysml; round trips: every.sysml/.kermlunder examples/), with the same minimum-count assertions. Output does not depend on the document name, which a scratch run over all 842 converting examples confirmed (byte-identical Turtle and api-json underrel,Base(path)).Stayed separate, and why
TestSymbolStructureMatchesGraphStructureandTestDeclaredSubjectsAreMinted(internal/translate/export): they callmodelToRDF,newEncoderand the identity tables, unexported.TestExamplesCorpusInvariants/TestAPIJSONWritersNeverEmitNameReferences(tests/export): already one conversion per model through that package'sconversionCache; folding them in would move ~2 000 lines of api-json helpers across packages — a follow-up.TestElementPropertiesServeEveryRuleAndExplainEveryOmission(tests/grpc): walks the same examples but its witnesses come from the server, not the export.TestIncrementalEqualsFreshCorpora,TestParallelBatchValidationMatchesSerial,TestExprTypeCheck*,TestModelsOverASharedLibraryAgreeWithModelsOverTheirOwn,TestLibraryRootNameCorpus,TestCommandLineLoadMatchesWorkspace, …): their property is a workspace pipeline (incremental vs fresh, batch vs serial, shared vs own index), nothing of parse → export to share.Shards
corpus-roundtrip,corpus-apijsonandexport-settledbecomecorpus-sweep(RACE_CORPUS_SWEEP := ^TestExamplesSweep$); 18 race shards, 17 CircleCI nodes, 9 quick-tier groups.scripts/race-shard.sh --sweeps(develop's, from the queue-tier work) namescorpus-sweepin place of the three, so it printscorpus-sweep export-corpus export-examples: the shardsmake test-queueruns race-free over the construct-cover subset.tests/hygiene TestSweepShardsAreByNameRaceShardsalready checks the list. The sibling contract (OPENSYSML_CORPUS_SUBSET,tests/corpus/testdata/construct_cover.txt,--sweeps) is unchanged.Expectation files
Both round-trip files were re-recorded with their recorders; the only change is the
Regenerate with:line in each header (TestCorpusRoundTrip/TestCorpusAPIJSONRoundTrip→TestExamplesSweep). No verdict moved: 880 files, 835 stable, 7 whitespace-only, 38 refused in both. The structure differential expectation is untouched.When a downloaded corpus is absent locally, only the two ratchets stand down (announced with the
GATE NOT RUNbanner;-update-*refuses); settled collections, the gRPC agreement and the read-back still run over the committed models and fixtures, as their own tests did. WithOPENSYSML_REQUIRE_*set, absence fails as before.OPENSYSML_CORPUS_SUBSET(develop's construct-cover subset, merged in): the sweep iterates the listed files, loads every SysML document of a directory one of them is in so the directory model is still checked whole (as tests/export's sweep did withWantsAny), and the round-trip gates check only the visited verdicts and refuse to record under it, as develop'srundid.TestConstructCoverkeeps skipping when a corpus is absent locally.Timings (local, 8 vCPU, -race, corpora downloaded,
OPENSYSML_REQUIRE_*=1)Before, one test per package (
--- PASSseconds; peak RSS):TestSettledCollectionsAreWhatReconcilingWouldFind(tests/export,-parallel 2)TestElementPropertiesAgreeWithTheAPIJSONExport(tests/grpc)TestCorpusAPIJSONRoundTrip(tests/corpus)TestCorpusRoundTrip(tests/corpus)TestWithoutDerivedReadsBackEveryExample(modelsync)After, one test:
TestExamplesSweep1186 s, 11.1 GB (8 procs); withGOMAXPROCS=4(a CI runner's width) 2161 s, 7.3 GB; without-raceat 4 procs 223 s, 5.6 GB. Job-seconds under-racefall 52 %; the longest single shard grows from 1001 s to 1186 s because the sweep is one process where the old five ran on four nodes.TestSymbolStructureMatchesGraphStructureis unchanged (704 s here).What the consolidation saves and what it cannot (CPU-seconds summed over the 880 files,
-race, one instrumented pass on develop): read 0.07, parse 0.6,ToRDFWith389, hop-1 api-json 572, hop-1 Turtle 19 — about 980 CPU-s of shared pipeline per test that paid it (three did, plus modelsync over 56 files), i.e. roughly 3 000 CPU-s ≈ 6 min of wall at 8 workers, which is the saving observed. The rest is each property's own work and stays: the hop back and hop 2 of each round trip, one RPC per element for the agreement (1 986 791 pairs), and settled collections' ownModelToRDFWith(395 CPU-s per file pass) plus reconcile and two api-json writes (1 114 CPU-s), with the whole-directory models (the corpus directories as one model each) a further 156 + 239 CPU-s. Parsing is noise; the export and the api-json writer are the pipeline.Memory: the peak is the settled property's whole-directory models (
spec-examples/sysmland the pilot corpora as one model each), as before: the old shard peaked at 8.0 GB here with-parallel 2. The sweep checks a directory as soon as its last document is loaded (largest directories first, so they overlap the rest) in one of the-parallelslots the per-model workers load in, so-parallelbounds the models in hand as it bounded the old sweep's parallel subtests, and a CI class sizes the peak the same way:-parallel 410.4 GB / 1603 s and-parallel 310.0 GB / 2055 s on a four-proc box under-race; the GitHub runners (16 GB) run at the default; the CircleCI node runs with-parallel 3, which at the ~0.8x RSS CircleCI showed for the old shard lands at about 8 GB — the large class's limit. Maintainer decision: go-race-test may needresource_class: xlarge, or CircleCI's full scan can wait for PR 3's subset;-parallel 2fits the memory but a two-worker sweep would pass the 55-minute timeout. Alternatives measured and not taken: the directory model fully alone (2740 s, 7.9 GB) or beside one worker (2161 s, 7.3 GB) — the directory checks are ~1100 single-threaded CPU-seconds that then no longer overlap.GOMEMLIMITdoes not help: under-racemost of the RSS is the detector's shadow memory, outside the Go heap.How it was verified
On this head:
gofmt -l .(empty),go vet ./...,make lint,python3 scripts/check-doc-ids.py,python3 scripts/changelog.py check,go test -count=1 ./tests/hygiene,scripts/ci-changed-areas-test.sh, the two-line-comment gate over.go,.sh,.ymland the Makefile;go test -race -count=1 ./tests/grpc ./internal/translate/interop/modelsync;TestExamplesSweepunder-race(the runs above; 927 settled models checked, 629 of 667 examples converting with 73 825 elements / 1 986 791 pairs agreeing, both ratchets 880 files unchanged — the same figures the five tests logged on develop) and without.Checklist
make lintpasses locally;make testrun as the shards above plusgo vet ./...andtests/hygiene(the whole suite was not re-run end to end)changes/unreleased/load-once-check-many.changed.mdmake docs-countshas nothing to update)CircleCI memory
The whole corpus stays exhaustive on
mainand tags (OPENSYSML_CORPUS_SUBSETunset there).corpus-sweepleaves the 16-nodego-race-testjob for its owngo-race-sweepjob onresource_class: xlarge(16 GB, 8 vCPU,-parallel 8: 11.1 GB measured locally at eight, ~0.8x of that on CircleCI); it runs wherego-race-testruns,halt-if-verifiedincluded, so a develop push still skips it.tests/hygienecounts the sweep job's shard with the array when checking that the nodes coverscripts/race-shard.sh.Requested by: @HuiJun
🟢 Devin Review found nothing to fix · View findings · Follow Devin's work