Skip to content

bindings/python: +feat Import transitions in bulk and document wrapper ownership - #892

Merged
Adda0 merged 1 commit into
develfrom
issue-796-python-bulk-pr
Oct 16, 2026
Merged

Adda0 merged 1 commit into
develfrom
issue-796-python-bulk-pr

Conversation

@Adda0

@Adda0 Adda0 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Building an automaton from Python went one transition at a time through
Nfa.add_transition or Delta.add: one Python call, one argument conversion and
one sorted insertion per transition.

Delta gains two bulk entry points over the C++ batched import
Delta::add(std::vector&&): add_transitions(sources, symbols,
targets) taking three equal-length sequences, and add_transitions_from(triples)
taking an iterable of (source, symbol, target). Both convert and range-check the
whole input into C++ vectors before touching the delta, so invalid input raises
and leaves the automaton exactly as it was, and both release the GIL around the
native sort and build. The semantics match repeated add(): duplicates collapse,
states up to the largest mentioned one are allocated, and an import into a
non-empty delta merges.

Measured in this workspace, 200000 random transitions over 50000 states:
per-transition add_transition 120.5 ms, add_transitions 59.5 ms, a 2.02x
speedup, both producing identical automata. Peak RSS grows by 66 MB for the
per-transition loop and by 72 MB for the bulk import, which additionally holds
the three input sequences and the converted vector at the same time, so the
bulk path trades a little peak memory for half the time.

The two "TODO: Refactor into a generator" are resolved: iter_transitions_from(state)
streams the post of one state by reference, get_trans_from_state_as_sequence()
is a list of it, and iterate() is documented as the streaming form of
get_trans_as_sequence(). Return types are unchanged.

The ownership rules are now written down in the .pxd and .pyx docstrings and
each has a test: Nfa and Nft own the C++ automaton through a shared_ptr; a Delta
view holds an aliasing shared_ptr and keeps its automaton alive; the iteration
generators keep their owner alive; Transition, SymbolPost and Run own a raw heap
object.

The shared_ptr TODO in nfa.pxd is resolved by measurement rather than deleted
blindly: creating and dropping a Delta view costs 128 ns and a cheap method call
through thisptr.get() costs 82 ns, so the atomic reference count is not visible
next to the Python call overhead itself. The TODO is replaced by that number.

Left from #796 and not part of this commit: the Nft bulk import, which also has
to extend levels for new states (#763); the export/import round trip with
#755's transitions_as_arrays() and pickling, which do not exist on devel yet;
a lifetime test for noodle segments sharing one automaton; and a Python
benchmark kept in the repository. The figures above were not re-measured
after rebasing.

Part of #796.

Merge order: this PR is stacked on #883, which adds the C++ batched import
Delta::add(std::vector&&) it wraps. Merge #883 first; this PR's
base is #883's branch until then.

@Adda0
Adda0 force-pushed the issue-796-python-bulk-pr branch from d24bbf0 to c160bf5 Compare October 15, 2026 12:14
@Adda0
Adda0 changed the base branch from devel to issue-782-delta-builder-pr October 15, 2026 12:14
@Adda0
Adda0 marked this pull request as ready for review October 15, 2026 12:57
…r ownership

Building an automaton from Python went one transition at a time through
Nfa.add_transition or Delta.add: one Python call, one argument conversion and
one sorted insertion per transition.

Delta gains two bulk entry points over the C++ batched import
Delta::add(std::vector<Transition>&&): add_transitions(sources, symbols,
targets) taking three equal-length sequences, and add_transitions_from(triples)
taking an iterable of (source, symbol, target). Both convert and range-check the
whole input into C++ vectors before touching the delta, so invalid input raises
and leaves the automaton exactly as it was, and both release the GIL around the
native sort and build. The semantics match repeated add(): duplicates collapse,
states up to the largest mentioned one are allocated, and an import into a
non-empty delta merges.

Measured in this workspace, 200000 random transitions over 50000 states:
per-transition add_transition 120.5 ms, add_transitions 59.5 ms, a 2.02x
speedup, both producing identical automata. Peak RSS grows by 66 MB for the
per-transition loop and by 72 MB for the bulk import, which additionally holds
the three input sequences and the converted vector at the same time, so the
bulk path trades a little peak memory for half the time.

The two "TODO: Refactor into a generator" are resolved: iter_transitions_from(state)
streams the post of one state by reference, get_trans_from_state_as_sequence()
is a list of it, and iterate() is documented as the streaming form of
get_trans_as_sequence(). Return types are unchanged.

The ownership rules are now written down in the .pxd and .pyx docstrings and
each has a test: Nfa and Nft own the C++ automaton through a shared_ptr; a Delta
view holds an aliasing shared_ptr and keeps its automaton alive; the iteration
generators keep their owner alive; Transition, SymbolPost and Run own a raw heap
object.

The shared_ptr TODO in nfa.pxd is resolved by measurement rather than deleted
blindly: creating and dropping a Delta view costs 128 ns and a cheap method call
through thisptr.get() costs 82 ns, so the atomic reference count is not visible
next to the Python call overhead itself. The TODO is replaced by that number.

Left from #796 and not part of this commit: the Nft bulk import, which also has
to extend levels for new states (#763); the export/import round trip with
#755's transitions_as_arrays() and pickling, which do not exist on devel yet;
a lifetime test for noodle segments sharing one automaton; and a Python
benchmark kept in the repository. The figures above were not re-measured
after rebasing.

Part of #796.
@Adda0
Adda0 force-pushed the issue-782-delta-builder-pr branch from 12dcfff to a3d23c9 Compare October 15, 2026 18:04
@Adda0
Adda0 force-pushed the issue-796-python-bulk-pr branch from c160bf5 to 4efaf7c Compare October 15, 2026 18:05
Base automatically changed from issue-782-delta-builder-pr to devel October 16, 2026 05:54
@Adda0
Adda0 merged commit bbacd3c into devel Oct 16, 2026
25 of 26 checks passed
@Adda0
Adda0 deleted the issue-796-python-bulk-pr branch October 16, 2026 05:56
Adda0 added a commit that referenced this pull request Oct 16, 2026
…mata_str (#957)

`copy.copy(nfa)` and `copy.deepcopy(nfa)` raised "no default __reduce__
due to non-trivial __cinit__"; only the custom `deepcopy()` method
existed, and it does not copy the label. `Nfa.__copy__` copies the
automaton with the C++ copy constructor and shares the label;
`Nfa.__deepcopy__` also deep-copies the label and registers the copy in
the memo, so an automaton referenced twice in a deep-copied container
stays a single object. The alphabet is shared in both cases, as the C++
copy constructor does.

`Nfa.to_mata_str(alphabet=None)` binds `Nfa::print_to_mata`, as
`Nft.to_mata_str()` already does for transducers. Symbols are named
from the given alphabet, else from the automaton's own alphabet, else
printed as numbers.

Part of #755.

### Tests

`test_nfa.py`: `to_mata_str()` with numeric symbols, with an explicit
alphabet and with the automaton's own alphabet;
`copy.copy`/`copy.deepcopy` keep the states, the initial and final
states and the delta, are independent of the original, share/copy the
label, and a label that refers back to the automaton resolves to the
copy. Dropping the memo registration fails that last check.

No performance claim: these are missing features of the issue (items c).

### Overlap

No overlap with #880 or #895; rebased onto #892 (merged) without
conflicts. Independent of the other #755 PRs; all of them merge together
without conflicts.
Adda0 added a commit that referenced this pull request Oct 16, 2026
…rom the export

Exporting the transitions built one Python `Transition` object per
transition. `Nfa.transitions_as_arrays()` instead fills three parallel
int64 `array('q')` buffers (sources, symbols, targets) in one C++ pass
over the delta, in the order of `iterate()`. The pass holds the GIL: a
thread mutating the delta mid-pass would invalidate the iterators and
overrun the counted buffers.

`to_dataframe()` builds its columns from these buffers, wrapped with
`numpy.frombuffer` (numpy comes with pandas; pandas copies an
`array.array` element by element but an ndarray in one go). The columns
keep their int64 dtype.

`Nfa` could not be pickled ("no default __reduce__ due to non-trivial
__cinit__"), so it could not be passed to `multiprocessing` workers.
`Nfa.__reduce__` now pickles the number of states, the initial and the
final states, the columnar export and the alphabet; the rebuild imports
the transitions with `Delta.add_transitions` (#892). The label is the
pickle state, restored by `Nfa.__setstate__` after the automaton is
memoized, so a label referring back to the automaton round-trips.
Pickling an automaton whose alphabet cannot be pickled raises
`TypeError`, as before, instead of silently dropping the alphabet.

Part of #755.
Adda0 added a commit that referenced this pull request Oct 16, 2026
… once (#958)

`Delta.state_post()`, `Delta.get_transitions_from()` and
`Nfa.get_transitions_from_state()` copied the whole `StatePost` and
then its `to_vector()` before converting it to Python objects. They now
read the post through a const pointer into the delta; the two
`SymbolPost` getters share one helper.

`Segmentation.get_segments()` and `get_untrimmed_segments()` copied the
segment vector by value, then each segment, then assigned its initial
and final states and its delta into a fresh `Nfa`, which also dropped
the segment's alphabet. The getters are now declared to return the
const reference the C++ API returns, and each segment is copied once
into the wrapping `Nfa`, alphabet included.

Part of #755.

### Measurements

Release bindings, `taskset -c 13`, each value a median of 7 calls, 3
alternating runs, median shown. Posts: one state with 2000 symbol posts
of 8 targets each. Segments: a segment automaton of 4 segments with 5000
states each.

| | devel | this branch |
|---|---|---|
| `Delta.state_post()` | 0.94 ms | 0.68 ms |
| `Nfa.get_transitions_from_state()` | 0.96 ms | 0.59 ms |
| `Delta.get_transitions_from()` | 5.18 ms | 3.17 ms |
| `Segmentation.get_segments()` | 7.60 ms | 1.33 ms |
| `Segmentation.get_untrimmed_segments()` | 33.06 ms | 10.12 ms |

Measured on devel `ba4a338a78`; the commits since (#948, #910, #890,
#883, #892, #944) do not change these code paths.

### Tests

`test_strings.py`: segments (trimmed and untrimmed) keep the alphabet of
the segmented automaton and are independent copies; this fails on
`devel` (`segment.alphabet` is `None`). `test_trans.py`: state posts and
transitions from a state with several targets per symbol, from a state
without transitions, and from a state past the delta.

### Overlap

- #892 (merged) already reads posts in place in
`get_trans_from_state_as_sequence()` and `iter_transitions_from()`; this
PR does the same for the three getters #892 left copying.
- No overlap with #880 or #895. Independent of the other #755 PRs; all
of them merge together without conflicts.
Adda0 added a commit that referenced this pull request Oct 16, 2026
…rom the export (#956)

Exporting the transitions built one Python `Transition` object per
transition. `Nfa.transitions_as_arrays()` instead fills three parallel
int64 `array('q')` buffers (sources, symbols, targets) in one C++ pass
over the delta, in the order of `iterate()`. The pass holds the GIL: a
thread mutating the delta mid-pass would invalidate the iterators and
overrun the counted buffers.

`to_dataframe()` builds its columns from these buffers, wrapped with
`numpy.frombuffer` (numpy comes with pandas; pandas copies an
`array.array` element by element but an ndarray in one go). The columns
keep their int64 dtype.

`Nfa` could not be pickled ("no default __reduce__ due to non-trivial
__cinit__"), so it could not be passed to `multiprocessing` workers.
`Nfa.__reduce__` now pickles the number of states, the initial and the
final states, the columnar export and the alphabet; the rebuild imports
the transitions with `Delta.add_transitions` (#892). The label is the
pickle state, restored by `Nfa.__setstate__` after the automaton is
memoized, so a label referring back to the automaton round-trips.
Pickling an automaton whose alphabet cannot be pickled raises
`TypeError`, as before, instead of silently dropping the alphabet.

Part of #755.

### Measurements

Release bindings, 1 000 000 transitions (50 000 states, 64 symbols),
`taskset -c 12`, each value a median of 5 calls, 3 alternating runs,
median shown:

| | devel | this branch |
|---|---|---|
| `to_dataframe()` | 1236 ms | 37 ms |
| `transitions_as_arrays()` | — | 35.1 ms |
| `list(iterate())`, per-transition objects, for comparison (code
unchanged; the gap is run-to-run noise) | 286 ms | 163 ms |

Column dtypes are `int64` before and after. Measured on devel
`ba4a338a78`; the commits since do not change these code paths (#957
adds `copy` support and `to_mata_str` next to them). The fill now holds
the GIL instead of releasing it, one release/acquire pair less per call,
so the single-threaded figures stand.

### Tests

`test_nfa.py`: the columnar export equals `iterate()` element by element
and has typecode `q` (also on an automaton with no transitions);
`to_dataframe()` keeps three int64 columns; a pickle round trip keeps
the number of states (including an isolated state), initial and final
states, the delta and the label, and the restored automaton is
independent; a label that is the automaton itself round-trips to the
restored automaton (`RecursionError` before the label moved to the
pickle state); an unpicklable alphabet raises `TypeError`; an `Nfa`
passes to a `spawn` multiprocessing worker. Replacing the pickled state
count with 0 fails the round-trip and the multiprocessing test.

### Overlap

- Builds on #892 (merged): the unpickling rebuild uses its
`Delta.add_transitions`.
- #880 changes `to_networkx_graph`, so this PR leaves that function on
`iterate()`.
- Rebased onto #957 (merged), which adds `__copy__`/`__deepcopy__` next
to `__reduce__`; independent of the other #755 PRs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant