Repository navigation
fix(analyzer): walk each type once when deciding which types hold NDArrays - #632
Merged
Merged
Conversation
…rrays OwnershipModel.HoldsCore memoized a "holds NDArrays" answer only when its walk had cut no cycle, and HolderMembersOf put the asking type itself on the walk. In a model whose types reference each other (entities with navigation properties) nearly every answer involves a cycle, so nothing was memoized and every type re-walked every path through the graph - exponential in the number of types. NDArrayHolderAnalyzer (NDW016/NDW017) runs this for every named type of the compilation. Seen in a consumer that references NumSharp only indirectly: NDArrayHolderAnalyzer took 321.6 s of analyzer time on one project (99 % of all analyzers, a 108 s compile) that declares no NDArray field at all; with this change 0.025 s and a 5 s compile, with the same diagnostics (none). HoldsCore now runs Tarjan's strongly connected components over the "stores a value of" graph: each type is visited once per walk, a true answer is memoized as soon as it is found, and the types of one cycle are memoized together with the answer of the type the walk entered the cycle through - "holds" is the least fixed point of an OR over the graph, so a cycle holds exactly when its entry does. HolderMembersOf answers a source class/struct that does not hold NDArrays as a whole from that memo instead of walking its members; for a holding type it keeps the type on the walk's stack as before, so a member that reaches arrays only through the type itself still adds nothing. Two tests pin it: a model of 14 types that all reference each other completes (no NDArray: clean; one NDArray: all 14 flagged NDW016). Both time out after 60 s without the change. All 330 tests of NumSharp.Tests.Build.Analyzer pass on net8.0 and net10.0.
Member
|
Hey @campersau, appreciate your issue openning and fix. |
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…pe is not a holder again, the ownership walk keeps its own stack, and every answer is order-independent - review of PR #632 PR #632 (campersau, 4ba7ed7) replaced OwnershipModel.HoldsCore's per-path walk with Tarjan's strongly connected components, which removes the exponential walk over cyclic domain models (321.6 s -> 0.025 s of NDArrayHolderAnalyzer time on the reporter's project). Reviewing it turned up four problems in the same code; this commit fixes all of them on top of it. 1. Regression: back-references became holders (new NDW017 false positives). HolderMembersOf's new fast path calls HoldsNDArrays(type) first, which memoizes the whole cycle as "holds", and HoldsCore consults that memo BEFORE its "this type is on the walk's stack" check. So a member whose type reaches arrays only by coming back to the declaring type - the rule the fixture states for CycleWithArrayB ("its back-reference to A adds nothing") and the PR description says it keeps - was reported as a holder: - NumSharp.Core's own build gained two NDW017 that master does not produce: NDScope._parent (Backends/NDScope.cs:65; the fix the warning asks for would dispose the parent scope from the child) and NDIterator._nestedChild (APIs/np.nditer.cs:137); - Node._next, TreeNode._parent, the member closing a ring, Base._d of a Base/Derived cycle and a wrapper pointing back at a source NDArray subclass all became holders, and NDW016 texts changed ("stores NDArrays in '_next' and '_v'" where master says "'_v'"). The existing tests could not see it: the exact-match gate compares line and id, never the member list in the message. 2. Pre-existing on master: an answer depended on what had been memoized first. Master memoized a ROOT answer (a full-graph answer, which a back-reference makes "holds") and HolderMembersOf trusted any memo entry, so whether TreeNode._parent was a holder depended on whether something had asked HoldsNDArrays(TreeNode) before - DisposeScan.HandOff does, from another type's Dispose body, on another analyzer thread. The same code could draw different NDW017s build to build. 3. Pre-existing, made worse by the PR: the walk recursed (HoldsCore -> Compute -> HolderMembersOf -> HoldsCore) and a deep type graph overflowed the thread's stack, which kills the compiler process. Master crashed on a 2,000-type chain (1,500 survived), the PR already at 1,500. A random entity model of 2,000 types with three navigation properties each - the PR's own scenario, a little larger, storing no NDArray at all - crashed the PR build; so did a 2,000-entity project with ordinary service code; a 3,000-type random model crashed master and the PR alike. 4. The PR's components were not exact strongly connected components: Compute stops at the first holding successor, so a component can be cut short. Harmless for "holds", wrong for the member rule below. The design now: - Expand(type) is Compute's rules as a node definition: "holds by itself" plus the successor list ("stores a value of" edges). A SOURCE NDArray subclass keeps its member edges (its members can lead back to it); NumSharp's own metadata NDArray family is not expanded. - ComponentOf(type): Prefetch expands every undecided type the question reaches, WITHOUT the lock, so Roslyn's binding of members, member types and attributes runs on every analyzer thread that asks at once; then one iterative Tarjan pass (explicit stack, every successor followed) decides each component under one lock, from the cached expansions, and publishes one Component object for all its types; readers stay lock-free. (Taking the lock around the binding first serialized it onto one thread: holder-analyzer thread time 3.3-5.3 s on a 1,000-entity model.) - HoldsNDArrays = the component's answer: the least fixed point of the OR, as before. - HolderMembersOf(T): empty when T does not hold; otherwise a member type in another component holds on its own answer (it cannot reach T: T reaches it, so reaching back would merge the two components), and a member type inside T's component holds unless T DOMINATES it on the component's reversed edges rooted at its anchors (the types that hold by themselves or through a type outside the component) - "every path from it to arrays passes through T". The dominator tree (Cooper-Harvey-Kennedy) is built lazily, once per holding component of two or more types, so each member is then an interval test. This is master's intended rule, now exact and independent of order and threads. Measured (Release, net10.0, the in-process harness, compile included): a 12,000-type chain 3.1 s and a 5,000-type random cyclic model 3.6 s (no array) / 4.9 s (one array), where master and the PR crash; single-threaded, the walk over 2,000 entity types takes 0.26 s once Roslyn has bound them, the dominator tree 12 ms and every type's HolderMembersOf together 12-45 ms. On a 1,000-entity project with service code (one file per class) the holder analyzer's thread time is 0.83 s against the PR's 1.42 s, wall time equal within noise. NumSharp.Core's own NDW set is master's again (2 NDW012, 0 NDW016/NDW017; the PR's build had 2 NDW017). Tests (all new files; 352/352 analyzer tests pass on net10.0): - OwnershipCycleTests (20): the holder rule member by member - self-reference, parent pointer + child list + sibling array, NumSharp's NDScope and NDIterator shapes, every container of the owner's own type, a ring, the HolderTypeScenarios cycle pair (its message, which the line gate cannot see), a base/derived cycle, a source NDArray subclass with a wrapper pointing back, mutual holders, a parent/child owner, a gate other than the owner, two routes, two anchors; the model queried after 200 random warm-up orders and from 16 threads (identical answers); a 10,000-type chain and 3,000-type random entity models with and without an array. - OwnershipGraphOracleTests (2): 630 seeded random type graphs (cycles, self-references, base types, List/array/tuple/Box/auto-property edges, borrowed/delegate/comparer/static/computed/ scalar non-edges); every type's exact holder set (NDW017 per holder of a disposable type, the NDW016 member list otherwise) against an independent breadth-first oracle of "reaches an NDArray without passing through the declaring type". Teeth: against the PR's commit 15 of the 19 tests that do not need a deep graph fail, and the 10,000-type chain and the 3,000-type entity model crash the test host (stack overflow); against master only the order-independence test fails, and the deep ones crash the same way. Mutants of the new code - the dominator test always true, no member edges for a source NDArray subclass, no root edges into the anchors, no low-link update on a back edge, outside holding successors not anchoring - are killed by 11, 1, 19, 13 and 40 tests. Docs: numsharp-build-compiler.md (NDW016) gains a "Back-references" paragraph; COVERAGE_PLAN.md replaces the obsolete "only root HoldsNDArrays answers are memoized" guardrail and lists the tests. (cherry picked from commit ce72b3f409863b80c61754ed86037e6cb6e9705e)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…emoized type lookup per operation - IsOwnedNDArrayType is memoized per type in the compilation's OwnershipModel, and a method body without a single owned-type operation ends after that one walk Found while reviewing PR #632 for problems like its exponential walk: the cost that every project referencing NumSharp pays whether or not it uses NDArray. NDArrayLeakAnalyzer runs an operation-block action on EVERY method body of such a project. Each body not exempted by an attribute was walked three times (the NDScope.Open exemption scan, CollectLocals, pass 2), and IsOwnedNDArrayType ran for every operation of pass 2 and every local assignment: two base-chain walks and three scans of the type's AllInterfaces (about thirty generic-math interfaces for an int, more for a string) - for the same few thousand distinct types, again and again. - OwnershipModel.OwnedValueTypes memoizes the answer per type for the compilation (it is a pure function of the type there; two callbacks computing it first store the same answer). The old body is now ComputeIsOwnedNDArrayType. - BlockScan.Run first asks AnyOwnedValue: does any operation of the body have an owned type? If none does, the body can report nothing and Run returns before the attribute check and the other walks. The exit is exact, not a heuristic: NDW012 is the analyzer's only diagnostic, pass 1 reports an owning local and RegisterSource makes a local owning only for a value with ProducesOwnedNDArray(value), which starts with IsOwnedNDArrayType(value.Type); pass 2 reports ProducesOwnedNDArray(op) itself. Both values are operations of the body, which AnyOwnedValue visits (lambda and local-function bodies included). Measured with CompilationWithAnalyzers' per-analyzer execution time (summed over threads) on a generated project that references NumSharp and never uses NDArray - 2,000 entity classes (an interface, IEquatable, three navigation properties, Equals/GetHashCode/ToString) and 500 service classes of 10 LINQ/StringBuilder/dictionary methods, one file per class; three interleaved runs each: NDW012 alone, before 116.4 - 136.5 s memo only 19.5 - 30.0 s memo + early exit (this) 9.2 - 14.9 s an analyzer that only reads every operation's Type once 2.7 - 5.9 s an empty operation-block action 0.04 - 0.28 s What remains is about three bare walks: the one walk plus the memo lookups. Wall time of a leak-only analysis is dominated by the compilation and was too noisy on this host to quote (the empty analyzer's own runs took 10-22 s). Behavior-preserving: the 352 analyzer tests pass (net10.0), and NumSharp.Core's NDW set is unchanged (the two NDW012 master reports, poly1d.cs:136 and NDArray.Indexing.cs:46). (cherry picked from commit 3644fe86c60732ff18ea55da0ea227785cbb1f9d)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…on once per method - a class implementing a k-deep interface chain cost 2^k searches per virtual method (56 s to weave at 26 levels), now linear; the memory is keyed by the instantiation as seen from the method's own type, so IBase<int> and IBase<string> are still both searched Found while reviewing PR #632 for problems like its exponential walk: the weaver has the same shape in its own inheritance walk. Not touched by the PR; master has it. NumSharp.Build's ScopeInheritance.FindNearest climbs a method's override/implementation graph to the nearest declaration carrying a scope-family attribute (EffectiveScope) and, separately, the nearest declaring [NDScopedExit] parameters (EffectiveExitParameters). CollectTargets asks both for EVERY method of every type in the module being woven - NDArray or not - and the walk runs for each virtual one; a public one also searches the interfaces its type lists. The C# compiler lists an interface's WHOLE base-interface closure on the interface itself and on every type implementing it. MatchInterface recursed into every listed base interface without remembering what it had searched, so a chain I0 <- I1 <- ... <- I(k-1) was searched once per path: 2^k searches for each virtual method of each implementing type, with no early exit when the method inherits nothing (the common case: an unscoped member). The fix: one map per walk from an interface instantiation to the shallowest depth it was searched at. A search is skipped when the same instantiation was searched before at the same or a shallower depth. The result cannot change: the walk is depth-first and returns on its first match, so the earlier search found nothing; every declaration it could test is already in `visited`; and a search from a shallower depth had at least as much of MaxDepth's budget. The key is the INSTANTIATION, not the definition: the interface's full name with each generic argument read through the GenericContext chain down to the terms of the method's declaring type (`!n` for its type parameters, `!!n` for method parameters; arrays, pointers, by-refs, pinned and modreq/modopt kept). IFoo<int> and IFoo<string> reached through two bases get different keys, since their members compare differently against the signature; the same instantiation reached along two paths gets one key. A type nested deeper than MaxDepth gets no key and is searched without the memory (never wrong, only slower). Measured (Release, the in-process weaver harness, compile excluded): a class implementing a k-deep chain, one unscoped member per level, plus an [NDScoped] method k = 20: 1.1 s -> 0.40 s; 22: 3.3 s; 24: 13.7 s; 26: 56.3 s -> 0.29 s; 40: 0.45 s; 60: 0.74 s a lattice of interface diamonds (each level inherits both interfaces of the level below) 12 levels: 1.36 s -> 0.33 s; 13: 3.35 s -> 0.31 s; 14: 9.38 s -> 0.32 s (about 3x per level before, so roughly ten minutes at 18) Tests: - WeaverInterfaceWalkTests (3): a 40-deep chain with an unscoped member at every level and an 18-level diamond lattice weave within 30 s (with a 120 s [Timeout]) and inherit [NDScoped] only for the root member; a generic interface under two instantiations (IBase<int> and IBase<string> through two interfaces), one instantiation reached twice (IBase<double>), and an implementer's own type parameter (Open<U> : IBase<U[]>) are all woven as before. - WeaverAnalyzerParityTests gains the InterfaceLattices fixture: for every method, the weaver's resolution agrees with the analyzer's, which resolves through Roslyn's own deduplicated AllInterfaces. Teeth: on the old walk both cost tests run into their timeout (12 levels of lattice still passed there in 1.4 s, hence 18); a mutant keying by the definition (IBase`1) fails OneGenericInterface_UnderTwoInstantiations_BothImplementationsInherit, because M(string) is left unwoven. 356/356 analyzer tests pass (net10.0). (cherry picked from commit 7b4614e34a74c7454df8ff8cebdccb9ec9adb71e)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…pays for finding its getters' properties - OwnScopeKinds was quadratic in a type's property count for every type the weaver loads (12 s for a class of 50,000 auto-properties with nothing to weave, now 1 s) Found while reviewing PR #632 for super-linear walks; the weaver code is not touched by the PR and master has it. ScopeInheritance.OwnScopeKinds is asked for every method of every type in the module being woven (EffectiveScope, and FindNearest for each declaration it visits). For a method without its own scope attribute it resolved a property-level attribute like this: if (m.IsGetter) foreach (var p in m.DeclaringType.Properties) if (ReferenceEquals(p.GetMethod, m)) return KindsOf(p.CustomAttributes); That is quadratic in the type's property count twice over: - the scan visits every property for every getter; - Mono.Cecil reads accessor semantics on first use (MethodDefinition.IsGetter, PropertyDefinition.GetMethod) by searching the type's properties linearly for each accessor (MetadataReader.GetMember). Cecil's read alone, measured on a type of N auto-properties: 1.0 s at 20,000, 10 s at 50,000 - a generated resource, DTO or interop class pays it. Now PropertyOfGetter answers from a per-type map in a ConditionalWeakTable (weakly keyed, so a map dies with its module): when no property of the type carries a scope-family attribute - nearly every type - the map is a shared empty one, and neither the scan nor Cecil's semantics read happens (property attributes are read without semantics). When one does, the map of getters to properties is built once per type. The answer is the old one: MethodDefinition keeps reference equality, the first property naming a getter wins as the scan's first match did, and a type with no attributed property can only ever have answered None. The weaver mutates no attributes, so the map cannot go stale. Measured (Release, the in-process weaver harness, weave only, two runs each), a class of N auto-properties: nothing to weave (no target) 20,000: 2.2 - 2.3 s -> 0.45 s 50,000: 11.9 - 12.4 s -> 0.9 - 1.0 s one [NDScoped] method elsewhere 20,000: 3.7 s -> 3.4 s 50,000: 15.2 s -> 12.2 s What is left in the second row is Mono.Cecil's WRITER, which reads every accessor's semantics through the same linear search when it rewrites the module. That one is Cecil's code and not fixed here. Tests: behavior-preserving; the 356 analyzer tests pass (net10.0). The property-level attribute tests are its gate: a mutant whose PropertyOfGetter always answers null fails 7 of them. (cherry picked from commit b8df8d29f6e3c957d30fda859e49d3dbee65a1cf)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…ML documentation - the 22 tests of OwnershipCycleTests/OwnershipGraphOracleTests say what each pins and why it matters, their helpers document parameters, returns and the assertion failures they raise, OwnershipModel's Component fields and its older helpers are documented, and the parity test that gained the InterfaceLattices row is too
Documentation only; no code changes. The test project's convention is undocumented test methods,
but this work's rule is that every member it writes or edits is documented:
- OwnershipCycleTests: each test's summary names the shape, the rule it pins and, where it applies,
what PR #632's commit or master did wrong on it (the NDScope/NDIterator NDW017s, the '_next'
member list, the 1,500/2,000-level stack overflows, the order-dependent memo); Findings' members,
Analyze, BuildOrderModel, Snapshot, RandomEntityModel and the parsing regexes are documented.
- OwnershipGraphOracleTests: the Shape enum (which shapes are edges and why each non-edge is one),
the generator's Member/GraphType/Graph, Generate, the oracle (ReachesArraysAvoiding,
ExpectedHolders, EffectivelyDisposable, ReachesArrays), CheckBatch and both tests.
- OwnershipModel: the Component fields (_types, _position, _inside, _anchors, _treeLock, and
_enter/_exit with their publication order), _k, _sourceAssembly, _canNameNDArray, and the helpers
IsDisposableInterface, HasPatternDispose, CanNameNDArray, FamilyKey and Eq.
- WeaverAnalyzerParityTests.BothResolvers_AgreeOnEveryMethod_AndMatchTheirOwnOutputs.
Checked with -p:GenerateDocumentationFile=true: no CS1574/CS1584/CS0419 in any changed file.
356/356 analyzer tests pass (net10.0).
(cherry picked from commit bebf9d3e2ce223c72af22afe5ff4c853645e0410)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
… tests over the ownership model's holder rule and its API, NDW012's pay-for-play exit, the weaver's interface walk and its getter gate; each class checked against master, the PR's commit and 10 mutants of the review's code, and two tests strengthened where a mutant showed they did not test what they claimed
No product change. Every test documents what it pins and why it matters.
New and widened test classes (test/NumSharp.Tests.Build.Analyzer):
- OwnershipCycleTests.EdgeCases.cs (33, a partial of OwnershipCycleTests, which keeps its 20):
- back-reference shapes: doubly linked lists; nullable-annotated, jagged, nested and BCL containers
of the own type; a tuple carrying an array beside the own type (a holder); static, event and
computed self-references (no edges); an explicit backing field (the field is the edge, not the
property); partial classes (all parts judged together); interface-typed links; records with a
positional self-reference; structs holding arrays and lists of themselves;
- generics: an unconstrained tree payload (only the instantiation holds), a payload constrained to
NDArray, a curiously recurring base, another instantiation of the own generic (a holder), nested
types forming one cycle;
- [NDBorrowed]: a borrowed member cuts the cycle; a borrowed TYPE on a cycle holds nothing, proven by
a type outside the cycle storing it;
- several components: rings where every type holds its own array, two cycles joined by one edge,
base and derived pointing at each other;
- scale: a 2,000-type ring with one anchor, a 300-spoke star (one spoke holding / every spoke
holding, with the message's "and 297 more"), a 3,000-level inheritance chain with self-references
at every level, an 8,000-level chain through generic containers;
- the message names and counts only the holders; disposal and NDW012 contagion through a cycle;
unresolved types in a cycle (CS0246) do not crash, resolvable members are still judged;
- determinism: a hand-off in an earlier file does not change the holders of the type it asks
about (6 runs, sequential and concurrent), and a 40-file random project analyzed concurrently
equals its sequential run.
- OwnershipModelApiTests.cs (8): the model queried directly - null and array-free queries answer
false/empty; 42 type kinds (arrays, pointers, dynamic, object/string, delegates, the comparer /
equality / observer / progress / weak-reference families, nullable, constructed generics, borrowed,
interfaces, enums, tuples, carriers) hold as the rules say in any query order; type parameters hold
through their constraints; constructed generics answer for their arguments; holder members are
fresh lists in declaration order with field/property kinds; a type holding only through its base
holds but lists nothing; owning disposables on a cycle; repeated queries.
- OwnershipGraphOracleTests.cs (+2, now 4): 360 random graphs over nine more member shapes (pairs,
nested lists, dictionary values, jagged arrays, nullable tuples, Lazy, sequences, key/value pairs, a
pair beside an array) and generic bases, against the independent oracle; and 12 graphs of 100-300
types queried in random order straight through the model (HolderMembersOf and HoldsNDArrays). The
original 13 shapes keep their exact random draws, so the existing seeds test what they tested.
- LeakAnalyzerPayForPlayTests.cs (12): the early exit added by 3644fe86 (a body without an owned
operation ends after one walk) must still see every place an owned value hides - a leak inside a
lambda, a local function and an async lambda; a method whose only owned value is a tuple or an
owning disposable; a generic method over a constrained type; every member body kind (accessors,
operators, constructors, finalizers, indexers, expression bodies); nameof/typeof/default draw
nothing; owned shapes vs lookalikes; one leak among hundreds of methods; concurrent files over
shared types report exactly the sequential leaks.
- WeaverInterfaceWalkTests.cs (+9, now 12): the instantiation key edge by edge (nested generic
arguments, array ranks and jaggedness, nullable and tuple arguments, an instantiation read through a
generic base class); a 100-deep chain past the walk's depth limit (root member still inherited,
analyzer agrees); explicit implementations of a 40-deep chain; the [NDScopedExit] walk through a
deep chain (linear, the kept argument survives) and a diamond lattice; a getter through a diamond
lattice.
- WeaverGetterGateTests.cs (8): the gate of b8df8d29 changes cost only - one scoped property among
3,000 plain ones, a scoped indexer and its override, a static property, a derived type with 500
plain properties inheriting from its base's property, a property-level [NDScopedCovered] in the
middle stopping the inheritance, a method named like a getter, a generic type's property, and
three weaves of same-named types in one process (scoped / plain / scoped).
- WeaverAnalyzerParityTests.cs (+2 rows, now 7): InterfaceKeys (the key's edges on both resolvers)
and GetterShapes (the gate's shapes on both). Pairing a Roslyn method with its Cecil twin now
tries the full parameter type names first (List<int> vs List<string>, int[,] vs int[][]), falling
back to the old bare-name rule, and an indexer's inherited source is compared by its metadata name.
- AnalyzerTestHarness.cs: every analyzer run fails on an analyzer exception (AD0001). Roslyn reports a
crashing analyzer as a diagnostic, so a test expecting "no warning" passed on a crashed analyzer.
RunProjectAsync analyzes a multi-file project sequentially or concurrently and orders the results,
so two runs compare equal.
- COVERAGE_PLAN.md lists all of the above.
Teeth (each class run against older code, in worktrees):
- The PR's commit (4ba7ed7): 45 of the 92 new or touched tests that do not crash fail (the
back-reference regressions), and HoldsNDArrays(null) throws ArgumentNullException.
- master (3c1f376): 7 fail - NullAndArrayFreeQueries (NullReferenceException on null);
HoldingOnlyThroughTheBase, OwningDisposable_OnACycle, DirectModel_LargeRandomGraphs and
HolderMembers_DoNotDependOnWhatWasAskedFirst (answers depend on query order);
SourceNDArraySubclass_WrapperPointingBack_IsNotAHolder (a back-pointer reported as a holder);
RandomMultiFileProject (times out at 300 s - also when run alone, so not CPU contention).
- The 2,000-type ring and the 8,000-level generic chain overflow the stack on master and on the PR's
commit; the 3,000-level inheritance chain passes on master and fails on the PR's commit.
- The weaver before 7b4614e3/b8df8d29: the 100-deep chain, both [NDScopedExit] walks and the getter
lattice time out at 120 s. The explicit implementations of a 40-deep chain pass there in 0.41 s
(that path was always linear; the test's doc says so).
- LeakAnalyzerPayForPlay, WeaverGetterGate and the two parity rows pass on the old code: they pin
behavior the optimizations must keep.
Mutation (10 mutants of the review's code, each applied alone, built, run against the targeted
classes, reverted; scratch script):
- NDW012's early exit skips lambdas and local functions: killed by the lambda, local-function and
async-lambda tests;
- the model drops tuple elements: TupleCarryingAnArray, TypeKinds and the 4 oracle tests;
- the model drops the base-type edge: HoldingOnlyThroughTheBase, CuriouslyRecurringBase, the oracles;
- the model expands a borrowed type: TypeKinds, and BorrowedType_InTheCycle once strengthened;
- the model drops the non-holding generic families: TypeKinds and the oracles;
- HoldsNDArrays without its null guard: NullAndArrayFreeQueries;
- the weaver key drops a nested argument's arguments: NestedGenericArguments,
NullableAndTupleArguments, the InterfaceKeys parity row;
- the weaver key drops array ranks: ArrayRanksAndJaggedness, the parity row;
- the getter gate ignores a property-level [NDScopedCovered]: CoveredPropertyInTheMiddle, the parity
row;
- the getter gate caches by type name across weaves: SameTypeNameInThreeAssemblies once strengthened.
Two tests were strengthened because a mutant was killed only by some other test:
- BorrowedType_InTheCycle passed with borrowed types expanded: the reporting site skips an
[NDBorrowed] type on its own, and the owner's link to it is no holder anyway (the owner dominates
it). A third type outside the cycle storing the borrowed type now sees the model's answer.
- SameTypeNameInThreeAssemblies passed with the gate's map cached by type name: a getter's own
property attribute is found by ScopeWeaver.CollectTargets, which never asks the gate. An override
now inherits through the gate in each weave.
Correction to ce72b3f4's message: against master, two of its tests fail, not only the
order-independence test - SourceNDArraySubclass_WrapperPointingBack_IsNotAHolder fails too (master
reports the wrapper's back-pointer '_w' as a holder of the source NDArray subclass).
Tests: NumSharp.Tests.Build.Analyzer 430/430 on net10.0 and net8.0.
(cherry picked from commit 98cb36a070b05e7ff8ca4f5e1684e7d0b823ebda)
Nucs
added a commit
that referenced
this pull request
Nov 2, 2026
…a47710 (the DType review 0a96ddb / 9e75f36 / 8804843, the parallel test suite, the NDW012 / weaver work of PR #632, the commented NPTypeCode members, the .claude/rules split) meet the string dtypes: 18 conflicted files resolved, journey4's code adapted where it assumed the String lane is unused, strings' tests made safe for the parallel suite Why a merge, and why this merge base - The owner chose "Merge journey4 in" over rebasing strings onto journey4 (decision of 2026-10-31, plan docs/plans/np-layer-dtype-logic.md D17/C13): a rebase of strings' history over journey4 measured as huge, and strings' commits are cited by hash throughout the plans and release notes. - The tree is git merge-tree --write-tree --merge-base=8a477103 227bf2f 8e5324b, not the true merge base f608647. Every journey4 commit in f608647..8a47710 (18 commits) already has a ported twin on strings: dd9a2c1->07903e85 9ed3c70->f23c41f6 02eca6d->aafc0f02 338b4fd->02f137fd 6f11dc5->8de98557 34a1705->a911607a 985ae38->b3625001 9b47b83->ea8174aa 1046439->639dcdad 631ccde->5210b4a6 24d4a02->f7042911 e8982e0->d0b1b9b3 96ff0d9->1a680c7f 0c86440->ef039d9d 6821b6e->ce960e63 de614c8->a62ddb5a 2676e90->6067757a 8a47710->8b0dd3e0 With the true base git replays those 18 against their twins and reports 60 conflicted files; with 8a47710 it reports 18, and merging 8a47710 into strings with the true base changes nothing outside those 18 files (checked before this merge, and port_check classified every flagged line as later evolution on strings). The commit's second parent is 8e5324b, so the next merge of journey4 starts from 8e5324b. What comes in from journey4 (8a47710..8e5324b) - 79d890e StackedMemoryPool trims its excess scalar slots in one pass; 70047d5 multivariate_hypergeometric(count) keeps a large, lightly sampled urn sparse. - 59c9b99 NumSharp.Tests runs in parallel, one test per worker (method-level [assembly: Parallelize]). - 9804b05, 78b2f7b, a91f2e7, 6aa1156 NDW012's message; c544595, 62b7e29, aec100b, 0988fef, 1bd6ce3, 2848edd, 03ba7e5, 7af67dc the PR #632 ownership-cycle review (analyzer and weaver). - f9f5f57, 351aa7b NumPy's other type codes, commented out in NPTypeCode. - 0a96ddb, 9e75f36, 8804843 the DType review: np.dtype(str) reads names by the whole string, np.min_scalar_type of an array, BuiltinCastingImpl.casting no longer tears, np.dtype(null) / a null result_type operand are NumPy's None, np.issctype(np.int32) is True again, NPY_TYPECHAR is NumPy 2.4.2's header enum, NPTypeCode.ToTypeCode is the parser's answer, np.find_common_type is NumPy 1.26.4's search; the dtype oracle's C-enum header gate. - 27ce521, d2ae2be, 6171751, 229e559 their release-note lines; cef9f1a, 8e5324b the .claude/CLAUDE.md split into a 26 KB root plus 24 .claude/rules files. The 18 conflicted files - .claude/CLAUDE.md: journey4's root as is. Strings' additions to the old monolith since f608647 (the string dtypes, the np-layer plan's phases, their traps) move into the .claude/rules files in the next commit, as current state. - docs/releases/RELEASE_0.71.0.md: both sides' lines kept in their sections; the dtype-oracle line carries the regenerated corpora's counts (9,367 rows LLP64 / 8,327 LP64). - docs/website-src/docs/dtypes.md (12 hunks) and docs/website-src/docs/numsharp-build-compiler.md: both sides' text, journey4's NDW012 procedure beside strings' string-dtype sections; where the two described one behaviour, the text of the code this merge keeps. - src/NumSharp.Core/APIs/np.cs: strings' complex declarations with journey4's comment and np.complex_ documentation. - Creation/np.asanyarray.cs: strings' code (journey4's Converts.* change touched the typed mixed-scalar loop that strings had already replaced with its coercion path); journey4's documentation of device= and of the exceptions, written for strings' first overload: ValueError (device, a matrix too large), NotSupportedException (null a with a null dtype: NumPy's object array), TypeError (null a with an integer dtype: Python's int(None)). - Creation/np.dtype.cs: one parser from both. Strings' control-character branch (a character below '\x18' is a type number) and its S / U / c / T switch run first; then journey4's whole-string name lookup (NumPy's typeDict), with 'a' / 'bytes' / 'bytes_' naming strings' Bytes class. The refusal sets are what NumSharp cannot store: _unsupported_numpy_typechars {'F', 'O', 'V'}, _unsupported_numpy_names {complex64, csingle, object, object_, void}. The descriptor is the numbered, flexible or C-type class first (strings' CTypeClassOf), else DType.From(lane). NPY_TYPES gains NPY_NTYPES_ABI_COMPATIBLE = 21; the duplicate NPY_VSTRINGLTR is gone (kept once, under NumPy's "New non-legacy DTypes" with strings' documentation); NPY_TYPECHAR's remarks name the unstorable characters (complex64, void, object) and say the string characters name NumSharp's string classes. - DTypes/DType.cs: strings' (the C integer and long-double classes are lane-sharing classes of their own). - DTypes/PyScalarDTypeMeta.cs: strings' BigInteger-aware body with the merged documentation. - Logic/np.can_cast.cs: can_cast(NDArray from, DType to, casting) => can_cast(from?.dtype, to, casting), documented with its TypeError. - Logic/np.find_common_type.cs: journey4's NumPy 1.26.4 search for the public np.find_common_type paths; strings' PromoteTypeSequence stays, now documented, as the NumPy 2 result_type the internal helpers need: _FindCommonType_Array => PromoteTypeSequence(array_types) (NDIter's operand promotion) and _FindCommonScalarType => PromoteArrArr. NumPy 1.26.4's search is not NumPy 2's promotion, so the helpers keep the latter. - Logic/np.min_scalar_type.cs: strings' code with the merged documentation. - Logic/np.result_type.cs: params NDArray[] raises ValueError when empty and reads one null as NumPy's None (the default float64 descriptor); the two-array overload sends a null operand to DTypePromotion.ResultType before strings' fast path. - test/NumSharp.Tests/DTypes/DTypeOracleTests.cs: LongDoubleClasses checks strings' own classes (full attribute check where NumPy's and NumSharp's sizes match; num and char always); NPY_SORTKIND moved into EnumPorts; merged class doc. - DTypes/corpus/dtype_oracle.llp64.jsonl / dtype_oracle.lp64.jsonl: regenerated by the merged generator (9,367 / 8,327 rows), not hand-merged. - test/NumSharp.Tests/OpenBugs.Reductions.cs: strings' file (groups 10 to 15 as strings left them) with journey4's .claude/rules/reductions.md reference where its paragraph still exists (the other paragraph is a fixed group's). - test/oracle/gen_dtype_oracle.py: both sides' keyword arguments and probes. Journey4 code adapted to the string dtypes (auto-merged files whose text assumed journey4's model) - Backends/NPTypeCode.cs: journey4 calls the String member a vestigial NPY_UNICODE slot; on strings it is the one lane of bytes_, str_ and StringDType. The enum's header comment and the commented Bytes / StringDType members say so (current state, not Stage D's plan); ToTypeCode's returns say the string characters ('S', 'a', 'c', 'U', 'T') come back as NPTypeCode.String and its exception list keeps only complex64, void and object; ToTYPECHAR refuses the String lane (DType.From(NPTypeCode.String) refuses: the lane names no one dtype). - Backends/Default/Sorting/SortKind.cs: NPY_SORTKIND is NumPy 2.4.2's header enum in its order and values, aliases and new-style names included (_NPY_SORT_UNDEFINED = -1, NPY_MERGESORT = 2, _NPY_SORT_HEAPSORT = 1, NPY_SORT_DEFAULT = 0, NPY_SORT_STABLE = 2, NPY_SORT_DESCENDING = 4). Journey4's C-enum header gate (8804843) lists NPY_SORTKIND as not ported and turns red for a NumPy enum that has a NumSharp mirror; strings has one (its sort kinds), so the enum moved to EnumPorts, where every member is checked against NumPy's header, and strings' mirror held three of its nine names. - Logic/np.type_checks.cs: np.issctype's DType arm (9e75f36) excludes NPTypeCode.String like the NPTypeCode and Type arms both sides share; its documentation no longer calls the String lane vestigial. NumPy 1.26.4's issctype is True for np.str_, np.bytes_, 'U5', 'S3', str and bytes (probed), so the three String exclusions are a strings bug, fixed in its own commit after this merge. - np.asanyarray mixed scalars: strings' boundary map stays the model (a C# integer is a Python int, a double a Python float, a Complex a Python complex; Half, char and decimal are NumPy scalars). gen_asanyarray_mixed_oracle.py writes that map, its corpus is regenerated (2,619 rows), and np.asanyarray.MixedScalars.Test.cs reads it (Integers_ArePythonInts_InEitherOrder replaces journey4's int32-before-int64 element-order case). - DTypeStringGrammarTests.cs, NpyTypeCharTests.cs: the characters and names that parse on strings ('S', 'a', 'c', 'U', 'T', 'str', 'bytes', ...) expect strings' classes instead of journey4's refusals (a new StringDTypeGrammar_ParsesNumPysSpellings pins NumPy 2.4.2's .str of each), and a control character read as a type number lands on strings' own class ('\x0d' the longdouble class, '\x12' bytes, '\x13' str; '\x11' object and '\x14' void stay refused). - AuditV2_CastingRandomUtilities.cs T1_65: 'c' maps to the String lane (NumPy's S1), never to char or complex. AuditV2_NDArrayCreation.cs T1_46: strings' fix stays (no [OpenBugs]; d0a6f4c made arr + 1000 on uint8 raise OverflowError) with journey4's np.find_common_type answer for the pair, int32 (NumPy 1.26.4's search). - np.type_checks.BattleTest.cs: np.dtype("String") is '<U0' on strings; its reason text says the string classes' lane, not the vestigial slot. - DTypeCasting.ParseCasting(null) is NumPy's TypeError "casting must be str, not NoneType" since journey4's 0a96ddb (strings' converter answered ValueError "(got '')"). Strings' ONNX Runtime Run tier (f1e2d7e) documented and tested ValueError for a null rule: the six Run / RunInto / RunAsync / RunIntoAsync forms now document the TypeError, and SessionRunModeTests.CastingRule_IsValidatedOnEveryCall_EvenWhenNoInputNeedsACast expects NumPy's TypeError for null on every form (ValueError stays for the unknown names). UnmanagedStorage.CanCastTypeCodes documents it too. - Logic/np.find_common_type.cs: strings' documentation of its old _can_coerce_all(List<NPTypeCode>) is gone with the method it described (journey4's 1.26.4 search replaced it and brought its own). Strings' tests under journey4's parallel suite (59c9b99) - [DoNotParallelize] with a reason on 22 strings test methods and the class StringDTypeFaultTests, the tests that flip a process-wide switch, read a process-wide counter, force collections or assert wall-clock bounds: np.nditer.NumPyParity, CharModuleTests, StringArithmeticTests, StringDTypeCastTests, StringDTypeIterationTests, StringDTypeStorageTests, StringRowsAndMaskTests, StringSearchTests, StringSortTests, TextNumberConversionTests, NpyStringDTypePickleTests, IndexSearchLayout. - 14 helpers read the caller's array through `using var c = np.ascontiguousarray(a)`: np.ascontiguousarray hands a C-contiguous operand back itself, so the helper disposed the CALLER's array. A serial run masked it (the freed buffer came back to the same test's next allocation); in parallel another test took the buffer (a net8.0 flake). Each helper now disposes only a copy np.ascontiguousarray made. Polynomial*Tests (12 sites) and np.average.TupleAxes.Test.cs (2). Layering ratchets (docs/plans/np-layer-dtype-logic.md R1 / R2), pinned by hand with their reasons - R2 engine_upcall_allowlist.tsv: Backends/NPTypeCode.cs 0/1, NPY_TYPECHAR.ToTypeCode's np.dtype(char) - NumPy's descriptor parser, the dtype layer's own code that stays on np (class D). - R1 facade_dtype_allowlist.tsv: Creation/np.dtype.cs 0/2 -> 0/3 (journey4's None arm, np.dtype(None) is float64); Logic/np.type_checks.cs 2/15 -> 3/15 (issctype's DType arm; back to 0 text lines with the issctype fix); Logic/np.find_common_type.cs 2/10 -> 2/5 (tightened: the 1.26.4 search has fewer typecode tests). Gates on this tree (Windows, i9-13900K) - dotnet build: 0 errors. - NumSharp.Tests, CI's configuration (Release, TestCategory!=OpenBugs&TestCategory!=HighMemory): net10.0 21,013 passed / 12 skipped, net8.0 21,012 passed / 12 skipped, none failed (the parallel suite of 59c9b99). - The same filter in Debug fails two tests on both frameworks, as strings' tip does and as the commits that added them do (ff8a2fc, 6b53594): ShortRows_SeveralNaNs_KeepTheRunningSumsNaN and Trapezoid_NaNSpacing_IsTheProductsFirstOperand. np.sum of [nan(1), 2.0, nan(2)] keeps nan(2) in a Debug build and NumPy's nan(1) in Release: the summation path that answers it leaves the NaN choice to the JIT's operand order. Not this merge's; pinned for a fix after it. - NumSharp.Tests.Oracle (Debug): 414 passed, 6 skipped on both frameworks; the 9 failures are the [OpenBugs] *_KnownGaps corpora, which CI excludes. Carved cells that now match NumPy: all 4 of MS22 (ma.frombuffer's buffer-size rule) and 12 of SU35's 13 (the dtype name 'cdouble'; the 13th now reaches finfo(cdouble).minexp, -1021 against NumPy's -1022, the finfo divergence of SU12-SU14) - the corpora are re-carved after this merge. - NumSharp.Tests.Build.Analyzer: 462 passed on both frameworks. - Interop: MLNet 207 passed / 1 skipped, ParquetNet 11, System.Numerics.Tensors 101, OnnxRuntime 289 passed / 22 skipped (with the casting adaptation above), on both frameworks. NumSharp.Tests.Interop 784 of 785: the failure, T1_47_CanCoerceAll_VariousStarts_NoThrow_Match_Numpy_ResultType, is journey4's own (8e5324b carries the same test and the same code): 8804843 made _can_coerce_all's start NumPy 1.26.4's index into the search ladder '?bBhHiIlLqQefdgFDGO', and the test still reads it as an index into the list (start 3 over [i4, f4, f8] is NumPy 1.26.4's float64, probed, not "empty"). Left for the next commits - The issctype String fix (NumPy 1.26.4: the string classes are scalar types). - T1_47 brought to NumPy 1.26.4's _can_coerce_all semantics. - The Debug-only NaN choice of np.sum's short rows: an [OpenBugs] pin, deferred (D21). - The oracle gap corpora re-carved (MS22, SU35). - Strings' .claude/CLAUDE.md content since f608647 ported into the .claude/rules files as current state; journey4's rules corrected where they describe journey4's model (the String lane, the C-type classes, np.asanyarray's mixed scalars, np.ScalarType). - The promotion helpers' move into the dtype layer, which waited for this merge (R2's 23 pinned calls).
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.
Problem
NDArrayHolderAnalyzer(NDW016/NDW017) can take minutes on projects whose types reference each other in cycles, even when those projects don't use NDArray at all. The analyzer ships in the NumSharp package, so it runs in every project that references NumSharp, directly or transitively.Seen in a project that references NumSharp only transitively and doesn't use NDArray itself, but contains a larger domain model whose types reference each other in cycles. With
-p:ReportAnalyzer=true:NDArrayHolderAnalyzer, 99 % of all analyzers)Both versions report the same diagnostics on that project (none).
Workaround until a release with this fix
ExcludeAssets="analyzers"on the package reference doesn't help: the SDK still passes the analyzer to csc, asbuild/NumSharp.targetsnotes. What works is removing it from@(Analyzer)before compiling, for example in aDirectory.Build.targets:This turns off all NDW diagnostics, so it only suits projects that use neither the NDArray ownership rules nor
[NDScoped].Cause
OwnershipModel.HoldsCorememoized a "holds NDArrays" answer only when its walk had cut no cycle.HolderMembersOfalso put the asking type itself on the walk. In a model whose types reference each other, nearly every answer involves a cycle, so almost nothing was memoized. Every type then re-walked every path through the graph, which grows exponentially with the size and density of the cycle. The holder analyzer does this for every named type of the compilation.Fix
HoldsCoreruns Tarjan's strongly connected components over the "stores a value of" graph, so each type is visited once per walk.trueanswer is memoized as soon as it is found. Finding one never depends on a cut cycle, so it is always exact.HolderMembersOfanswers a source class/struct that holds no NDArrays straight from that memo instead of walking its members.HolderMembersOfkeeps the existing behaviour: the type stays on the walk's stack, so a member that reaches arrays only by coming back to the type itself adds nothing.Tests
Two new tests in
OwnershipRobustnessTestsuse a model of 14 types that all reference each other:DenselyCyclicModelWithoutArrays_CompletesClean: no diagnostics.DenselyCyclicModelWithAnArray_FlagsEveryType: one type stores anNDArray, and all 14 are flagged NDW016.Both time out after 60 s on
masterand finish at once with this change. All 330 tests ofNumSharp.Tests.Build.Analyzerpass on net8.0 and net10.0.