diff --git a/.github/workflows/build-wheels.yml b/.github/workflows/build-wheels.yml index 8cf6f6a..995789f 100644 --- a/.github/workflows/build-wheels.yml +++ b/.github/workflows/build-wheels.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-22.04, macos-latest, macos-15-intel, windows-latest] - python-version: ['3.10', '3.11', '3.12', '3.13'] + python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] fail-fast: false steps: @@ -61,7 +61,10 @@ jobs: echo "PATH=${PCRE2_DIR}/bin;$PATH" >> $GITHUB_ENV - name: Build wheels - uses: pypa/cibuildwheel@v2.22.0 + # v4.x supports CPython 3.14 GA (cp314); CIBW_* env config below is + # still honoured. cp314-* matches the regular build, not cp314t + # (free-threaded), which we don't build. + uses: pypa/cibuildwheel@v4.1.0 env: CIBW_BUILD: ${{ steps.set-py-version.outputs.cibw-python }}-* CIBW_SKIP: pp* *musl* @@ -85,23 +88,67 @@ jobs: name: wheels-${{ matrix.os }}-py${{ matrix.python-version }} path: ./wheelhouse/*.whl + build_sdist: + name: Build sdist + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Checkout submodules + run: git submodule update --init --recursive + + - name: Install PCRE2 + # meson-python configures the build (meson setup + meson dist) to + # produce the sdist, so the build deps incl. PCRE2 must be present. + run: | + sudo apt-get update -y + sudo apt-get install -y libpcre2-dev + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Build sdist + run: | + python -m pip install --upgrade build + python -m build --sdist + + - name: Upload sdist + uses: actions/upload-artifact@v4 + with: + name: sdist + path: dist/*.tar.gz + publish: - name: Publish wheels to GitHub Release and PyPI - needs: build_wheels + name: Publish wheels and sdist to GitHub Release and PyPI + needs: [build_wheels, build_sdist] if: startsWith(github.ref, 'refs/tags/v') runs-on: ubuntu-latest steps: - - name: Download all wheel artifacts + - name: Download wheel artifacts uses: actions/download-artifact@v4 with: path: dist pattern: wheels-* merge-multiple: true - - name: Attach wheels to GitHub Release + - name: Download sdist artifact + uses: actions/download-artifact@v4 + with: + path: dist + pattern: sdist + merge-multiple: true + + - name: Attach wheels and sdist to GitHub Release uses: softprops/action-gh-release@v1 with: - files: dist/*.whl + files: | + dist/*.whl + dist/*.tar.gz fail_on_unmatched_files: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index e940d69..9433358 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -88,6 +88,13 @@ jobs: - name: Test ase-extxyz (incl. fextxyz Fortran round-trip) run: USE_FORTRAN=T pytest -v python/ase-extxyz/tests/ + + - name: Test with the first-char dispatch comment parser (use_cleri=False) + # Run the whole C-backend suite through the dispatcher so every fixture + # exercises it, in addition to the differential parity test. + run: | + USE_CLERI=false USE_CEXTXYZ=true pytest -v python/ase-extxyz/tests/ + pytest -v tests/test_dispatch_parity.py # Uncomment to get SSH access for testing # - name: Setup tmate session # if: failure() diff --git a/README.md b/README.md index 896e565..4840dea 100644 --- a/README.md +++ b/README.md @@ -57,21 +57,21 @@ forces, and a couple of `info` keys): | atoms / frame | file size | ASE built-in `extxyz` | `cextxyz` plugin | `extxyz.read_dicts` (no Atoms) | speedup, plugin / built-in | speedup, parser / built-in | |--:|--:|--:|--:|--:|--:|--:| -| 10 | 0.00 MB | 0.107 ms | 0.074 ms | 0.119 ms | 1.46× | 0.90× | -| 100 | 0.01 MB | 0.203 ms | 0.096 ms | 0.145 ms | 2.11× | 1.41× | -| 1 000 | 0.11 MB | 1.170 ms | 0.277 ms | 0.367 ms | 4.23× | 3.18× | -| 4 000 | 0.44 MB | 4.414 ms | 0.908 ms | 1.122 ms | 4.86× | 3.93× | -| 16 000 | 1.74 MB | 17.5 ms | 3.40 ms | 3.96 ms | 5.15× | 4.42× | -| 64 000 | 6.98 MB | 69.9 ms | 13.5 ms | 15.6 ms | 5.17× | 4.48× | -|200 000 | 21.80 MB |218.8 ms | 42.8 ms | 49.4 ms | 5.12× | 4.43× | +| 10 | 0.00 MB | 0.130 ms | 0.073 ms | 0.111 ms | 1.77× | 1.18× | +| 100 | 0.01 MB | 0.220 ms | 0.086 ms | 0.138 ms | 2.56× | 1.60× | +| 1 000 | 0.11 MB | 1.177 ms | 0.240 ms | 0.350 ms | 4.89× | 3.36× | +| 4 000 | 0.44 MB | 4.429 ms | 0.705 ms | 0.943 ms | 6.28× | 4.70× | +| 16 000 | 1.74 MB | 17.8 ms | 2.59 ms | 3.40 ms | 6.87× | 5.24× | +| 64 000 | 6.98 MB | 72.6 ms | 11.5 ms | 14.7 ms | 6.30× | 4.96× | +|200 000 | 21.80 MB |224.8 ms | 35.9 ms | 44.2 ms | 6.26× | 5.09× | ![Read-time benchmark](https://raw.githubusercontent.com/libAtoms/extxyz/master/benchmarks/read_speedup.png) Below ~100 atoms per frame the per-call setup (file open, PCRE2 JIT -compile, libcleri grammar walk for the comment line) is larger than -the regex match itself, so the built-in is faster on tiny files. From -~1 000 atoms upwards the parser dominates and `cextxyz` runs at a -steady ~5× over the built-in end-to-end (~4.4× for the parser alone). +compile, libcleri grammar walk for the comment line) is a larger share +of the work, so the margin shrinks on tiny files. From ~1 000 atoms +upwards the parser dominates and `cextxyz` runs at a steady ~6× over +the built-in end-to-end (~5× for the regex parser alone). The two cextxyz curves track each other closely: the `Frame → Atoms` translation in the ASE plugin is kept cheap by aliasing the parser's per-atom buffers directly into `atoms.arrays` (so `Atoms.__init__` @@ -97,14 +97,14 @@ The single biggest remaining cost is the per-line `pcre2_match`. The default per-atom lines are split on whitespace and each field is parsed and validated by its column type, with no regex compile or match. It is **the default since v0.4.2** (pass `use_regex=True` for the strict regex parser), and a further -**~1.5×** on top of everything above: +**~1.8×** on top of everything above: | atoms / frame | `read_dicts` (regex) | `read_dicts` (`use_regex=False`) | tokenizer / regex | tokenizer / built-in | |--:|--:|--:|--:|--:| -| 1 000 | 0.367 ms | 0.219 ms | 1.68× | 5.34× | -| 16 000 | 3.96 ms | 2.63 ms | 1.50× | 6.66× | -| 64 000 | 15.6 ms | 10.4 ms | 1.50× | 6.70× | -| 200 000 | 49.4 ms | 32.9 ms | 1.50× | 6.64× | +| 1 000 | 0.350 ms | 0.161 ms | 2.17× | 7.30× | +| 16 000 | 3.40 ms | 1.95 ms | 1.74× | 9.11× | +| 64 000 | 14.7 ms | 8.21 ms | 1.79× | 8.85× | +| 200 000 | 44.2 ms | 24.8 ms | 1.78× | 9.05× | It validates each field (a malformed numeric/bool or the wrong field count is a clear parse error, not a silent `0`) and is bit-identical to the regex parser on @@ -112,6 +112,36 @@ valid input. The trade-off is that it is marginally more lenient than the gramma on a few numeric edge cases (e.g. leading-zero integers `007`, `1.`/`.5`); pass `use_regex=True` if you need the grammar enforced exactly. +### Comment-line parser (`use_cleri=False`) + +The remaining per-frame cost is parsing the comment line. By default this walks +the libcleri grammar (PCRE2-backed). `use_cleri=False` (C backend only) instead +uses a hand-written first-char-dispatch parser that accepts the **same** language +— validated bit-identical against the grammar by a differential conformance test +(`tests/test_dispatch_parity.py`), with libcleri kept as the canonical grammar / +oracle / fallback — but builds the dicts in a single pass instead of constructing +and re-walking a generic parse tree. + +Because the win is per comment line, it is amortised away on single huge frames +(the tables above are unchanged) and grows as frames get smaller. Sweeping +atoms-per-frame at a fixed ~1 M total atoms (C reader, whitespace tokenizer in +both; only the comment parser differs): + +| atoms / frame | frames | `read_dicts` (cleri) | (`use_cleri=False`) | dispatch / cleri | full ASE read | +|--:|--:|--:|--:|--:|--:| +| 5 | 200 000 | 3.54 s | 2.00 s | 1.77× | 1.36× | +| 10 | 100 000 | 1.86 s | 1.07 s | 1.74× | 1.34× | +| 20 | 50 000 | 1.01 s | 0.613 s | 1.65× | 1.30× | +| 50 | 20 000 | 0.490 s | 0.332 s | 1.47× | 1.26× | +| 100 | 10 000 | 0.310 s | 0.231 s | 1.34× | 1.19× | +| 500 | 2 000 | 0.170 s | 0.153 s | 1.11× | 1.07× | +| 2 000 | 500 | 0.144 s | 0.135 s | 1.07× | 1.02× | + +It is the libcleri grammar (`use_cleri=True`) by default for now; pass +`use_cleri=False` for the dispatch parser. ~75 % of its speedup is from not +building/walking a generic cleri parse tree (the matching itself is a small part +of the cost), so it stays grammar-faithful while skipping cleri's machinery. + ### Marshalling in C Once the C reader has parsed a frame it has to hand the `info`/`arrays` data @@ -144,6 +174,8 @@ Reproduce locally (requires `extxyz`, `ase-extxyz`, `ase`, `matplotlib`): ```bash python benchmarks/bench_read.py --max-atoms 200000 --repeats 3 python benchmarks/plot_bench.py +# comment-line parser, many small frames (use_cleri table above): +python benchmarks/bench_cleri_frames.py --total 1000000 --repeats 3 # writing (see below): python benchmarks/bench_write.py --max-atoms 200000 --repeats 5 python benchmarks/plot_bench.py --in benchmarks/write_results.csv --out benchmarks/write_speedup.png @@ -157,11 +189,11 @@ than `extxyz-ng`): | atoms / frame | file size | ASE built-in `extxyz` | `cextxyz` plugin | `extxyz.write_dicts` (no Atoms) | speedup, plugin / built-in | speedup, writer / built-in | |--:|--:|--:|--:|--:|--:|--:| -| 1 000 | 0.11 MB | 2.800 ms | 0.639 ms | 0.547 ms | 4.39× | 5.11× | -| 4 000 | 0.44 MB | 10.9 ms | 2.391 ms | 2.163 ms | 4.55× | 5.03× | -| 16 000 | 1.74 MB | 43.9 ms | 8.426 ms | 7.273 ms | 5.21× | 6.03× | -| 64 000 | 6.98 MB | 166.6 ms | 31.6 ms | 28.2 ms | 5.26× | 5.92× | -|200 000 | 21.80 MB | 521.3 ms | 106.7 ms | 92.5 ms | 4.88× | 5.64× | +| 1 000 | 0.11 MB | 2.794 ms | 0.630 ms | 0.565 ms | 4.44× | 4.95× | +| 4 000 | 0.44 MB | 10.8 ms | 2.104 ms | 1.957 ms | 5.11× | 5.50× | +| 16 000 | 1.74 MB | 41.4 ms | 8.681 ms | 7.667 ms | 4.77× | 5.41× | +| 64 000 | 6.98 MB | 167.3 ms | 33.5 ms | 28.8 ms | 4.99× | 5.80× | +|200 000 | 21.80 MB | 509.7 ms | 102.6 ms | 88.2 ms | 4.97× | 5.78× | ![Write-time benchmark](https://raw.githubusercontent.com/libAtoms/extxyz/master/benchmarks/write_speedup.png) diff --git a/benchmarks/bench_cleri_frames.py b/benchmarks/bench_cleri_frames.py new file mode 100644 index 0000000..7ce79a3 --- /dev/null +++ b/benchmarks/bench_cleri_frames.py @@ -0,0 +1,92 @@ +"""Benchmark the comment-line parser: libcleri grammar (use_cleri=True, default) +vs the first-char dispatch parser (use_cleri=False). + +The dispatch parser's win is on the PER-FRAME comment line, so it is amortised +away on single huge frames (see bench_read.py) and shows up on files with many +small frames. This sweeps atoms-per-frame at a fixed total atom count and times +the C reader both ways (per-atom tokenizer in both; only the comment parser +differs). + +Run:: + + python benchmarks/bench_cleri_frames.py [--total 1000000] [--repeats 3] +""" +from __future__ import annotations + +import argparse +import csv +import sys +import tempfile +import time +from pathlib import Path + +import ase.io +import ase_extxyz.io # noqa: F401 (registers the 'cextxyz' format) +import extxyz + +from bench_read import make_xyz # reuse the synthetic-frame generator + + +def _best(fn, repeats): + best = float('inf') + for _ in range(repeats): + t0 = time.perf_counter() + fn() + best = min(best, time.perf_counter() - t0) + return best + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument('--out', type=Path, default=Path('benchmarks/cleri_results.csv')) + ap.add_argument('--total', type=int, default=1_000_000, + help='approx total atoms per file (held ~constant)') + ap.add_argument('--repeats', type=int, default=3) + args = ap.parse_args() + + per_frame = [5, 10, 20, 50, 100, 500, 2000] + + hdr = (f'{"atoms/frame":>11} {"frames":>7} {"MB":>6} ' + f'{"cleri":>9} {"dispatch":>9} {"speedup":>8} {"full rd speedup":>15}') + print(hdr); print('-' * len(hdr)) + + rows = [] + with tempfile.TemporaryDirectory() as tmp: + for nat in per_frame: + nframes = max(1, args.total // nat) + path = Path(tmp) / f'f_{nat}.xyz' + mb = make_xyz(path, nat, nframes) / 1e6 + sp = str(path) + + # parser only (read_dicts), per-atom tokenizer in both; comment parser differs + t_cleri = _best(lambda: extxyz.read_dicts(sp, use_cleri=True), args.repeats) + t_disp = _best(lambda: extxyz.read_dicts(sp, use_cleri=False), args.repeats) + # full ASE plugin read, both ways + t_cleri_full = _best(lambda: ase.io.read(sp, format='cextxyz', index=':', + use_cleri=True), args.repeats) + t_disp_full = _best(lambda: ase.io.read(sp, format='cextxyz', index=':', + use_cleri=False), args.repeats) + + speedup = t_cleri / t_disp if t_disp else float('nan') + full_speedup = t_cleri_full / t_disp_full if t_disp_full else float('nan') + rows.append(dict(atoms_per_frame=nat, frames=nframes, file_mb=mb, + read_dicts_cleri_s=t_cleri, read_dicts_dispatch_s=t_disp, + dispatch_speedup=speedup, + full_read_cleri_s=t_cleri_full, + full_read_dispatch_s=t_disp_full, + full_read_speedup=full_speedup)) + print(f'{nat:>11} {nframes:>7} {mb:>6.1f} ' + f'{t_cleri:>8.3f}s {t_disp:>8.3f}s {speedup:>7.2f}x ' + f'{full_speedup:>14.2f}x') + + args.out.parent.mkdir(parents=True, exist_ok=True) + with args.out.open('w', newline='') as f: + w = csv.DictWriter(f, fieldnames=list(rows[0].keys())) + w.writeheader() + w.writerows(rows) + print(f'Wrote {args.out}') + + +if __name__ == '__main__': + sys.path.insert(0, str(Path(__file__).resolve().parent)) + main() diff --git a/benchmarks/bench_read.py b/benchmarks/bench_read.py index a7183cd..5084008 100644 --- a/benchmarks/bench_read.py +++ b/benchmarks/bench_read.py @@ -82,12 +82,14 @@ def time_read_ase(path: Path, fmt: str, repeats: int) -> float: def time_read_dicts(path: Path, repeats: int) -> float: - """Time the ASE-free dict-based reader — what extxyz.read_dicts costs.""" - return _best_of(lambda: extxyz.read_dicts(str(path)), repeats) + """Time the ASE-free dict-based reader with the strict regex per-atom parser + (use_regex=True) — the ``read_dicts`` baseline in the README tables. The + package default is now the tokenizer (use_regex=False), timed below.""" + return _best_of(lambda: extxyz.read_dicts(str(path), use_regex=True), repeats) def time_read_dicts_fast(path: Path, repeats: int) -> float: - """As above but with the opt-in whitespace tokenizer (use_regex=False).""" + """As above but with the default whitespace tokenizer (use_regex=False).""" return _best_of(lambda: extxyz.read_dicts(str(path), use_regex=False), repeats) diff --git a/benchmarks/cleri_results.csv b/benchmarks/cleri_results.csv new file mode 100644 index 0000000..e1e8e6b --- /dev/null +++ b/benchmarks/cleri_results.csv @@ -0,0 +1,8 @@ +atoms_per_frame,frames,file_mb,read_dicts_cleri_s,read_dicts_dispatch_s,dispatch_speedup,full_read_cleri_s,full_read_dispatch_s,full_read_speedup +5,200000,147.8,3.54335541697219,2.001780458027497,1.7701019124063793,6.126253166003153,4.506593667087145,1.3593977222186266 +10,100000,128.5,1.8616587079595774,1.0704289170680568,1.7391707924508686,3.1345056670252234,2.3414170830510557,1.3387216185083561 +20,50000,118.75,1.0107304580742493,0.6128594169858843,1.6492044179481524,1.6579841249622405,1.2738971250364557,1.3015055080800133 +50,20000,112.9,0.48988604196347296,0.33220262499526143,1.4746603581788698,0.7712598750367761,0.6124645000090823,1.2592727823822263 +100,10000,110.96,0.3102357500465587,0.2310199160128832,1.3428961251516423,0.4610714999726042,0.3867935830494389,1.1920350289618722 +500,2000,109.404,0.1701008330564946,0.15335649996995926,1.1091856757934313,0.22910087497439235,0.21320445893798023,1.0745594914646521 +2000,500,109.1015,0.14380666706711054,0.1346410830738023,1.0680741998211958,0.1845930420095101,0.18073112494312227,1.0213683009365608 diff --git a/benchmarks/read_speedup.png b/benchmarks/read_speedup.png index 82587a5..3274f9e 100644 Binary files a/benchmarks/read_speedup.png and b/benchmarks/read_speedup.png differ diff --git a/benchmarks/results.csv b/benchmarks/results.csv index 54e8fa3..571d5c1 100644 --- a/benchmarks/results.csv +++ b/benchmarks/results.csv @@ -1,8 +1,8 @@ natoms,frames,file_mb,builtin_s,cextxyz_s,read_dicts_s,read_dicts_fast_s,speedup,parse_speedup,fast_speedup -10,1,0.001285,0.00010749998909886926,7.38749949960038e-05,0.00011920899851247668,5.129100463818759e-05,1.4551606955057577,0.9017774701598392,2.0958838661318113 -100,1,0.011096,0.0002034579956671223,9.629200212657452e-05,0.0001446249953005463,6.579099863301963e-05,2.112927254328761,1.4067969042579032,3.0924898526317466 -1000,1,0.109203,0.0011697909940266982,0.00027658400358632207,0.0003674159961519763,0.00021912499505560845,4.229423896026603,3.1838325121338245,5.33846444003266 -4000,1,0.436203,0.004413624992594123,0.0009084579942282289,0.0011222080065635964,0.0007287080079549924,4.858369919837265,3.932982982459232,6.056781240788457 -16000,1,1.744204,0.01751962500566151,0.0034028340014629066,0.003959790992666967,0.002632292002090253,5.148539422766341,4.424381245905565,6.65565408083507 -64000,1,6.976204,0.06988954100233968,0.013509042008081451,0.015597417004755698,0.010424208012409508,5.173537913386456,4.480840704651941,6.70454205433541 -200000,1,21.800211,0.21879487499245442,0.04276062498684041,0.04939795799145941,0.032940000004600734,5.116737069670721,4.429229140003777,6.642224497932463 +10,1,0.001285,0.00012995791621506214,7.329101208597422e-05,0.00011058303061872721,3.7458958104252815e-05,1.7731767172571538,1.1752066794329092,3.4693414550766075 +100,1,0.011096,0.00022045790683478117,8.595804683864117e-05,0.00013791699893772602,5.2332994528114796e-05,2.5647151714442815,1.5984824824554444,4.212598740481874 +1000,1,0.109203,0.001177040976472199,0.00024049996864050627,0.00035045796539634466,0.0001612498890608549,4.894141912474061,3.3585796092294373,7.299483945864853 +4000,1,0.436203,0.004429124994203448,0.0007049170089885592,0.0009425421012565494,0.0005173330428078771,6.28318644283888,4.699126954964413,8.561457760679518 +16000,1,1.744204,0.017779374960809946,0.0025887920055538416,0.003395583014935255,0.0019517079927027225,6.867826740297067,5.2360301257865,9.109649100831469 +64000,1,6.976204,0.07264966703951359,0.011525750043801963,0.014659667038358748,0.008212041924707592,6.303248531628651,4.955751508504059,8.84672383624009 +200000,1,21.800211,0.22477775008883327,0.03588125004898757,0.044165458995848894,0.02484933298546821,6.264490500803376,5.089446712417505,9.04562509666889 diff --git a/benchmarks/write_results.csv b/benchmarks/write_results.csv index 02cfff6..796ffb5 100644 --- a/benchmarks/write_results.csv +++ b/benchmarks/write_results.csv @@ -1,6 +1,6 @@ natoms,frames,file_mb,builtin_s,cextxyz_s,write_dicts_s,pypython_s,speedup,write_speedup -1000,1,0.109203,0.002799833018798381,0.0006385000015143305,0.0005474580102600157,0.0030485409952234477,4.385016463834012,5.114242492257255 -4000,1,0.436203,0.010870625003008172,0.002391167014138773,0.0021625419904012233,0.011302291008178145,4.546158816482104,5.026781006454034 -16000,1,1.744204,0.04386191698722541,0.008425584004726261,0.007273458002600819,0.04454758297652006,5.2058013975792585,6.03040767837546 -64000,1,6.976204,0.1665553750062827,0.031641958019463345,0.028150958998594433,0.1700303329853341,5.263750584076766,5.916508031381764 -200000,1,21.800211,0.5213367920077872,0.10673779097851366,0.09247166602290235,0.5287190410017502,4.884275636852297,5.637800360152139 +1000,1,0.109203,0.002793958061374724,0.0006298749940469861,0.0005647499347105622,0.0029531250474974513,4.435734213583189,4.947248135241741 +4000,1,0.436203,0.010762332938611507,0.002104375045746565,0.001956957974471152,0.010912708006799221,5.114265615515972,5.499521747021634 +16000,1,1.744204,0.04144945798907429,0.008681291015818715,0.007667165948078036,0.04373008303809911,4.774573034534458,5.40609897708874 +64000,1,6.976204,0.16732429200783372,0.03351095807738602,0.028831499977968633,0.17090941697824746,4.993121701308446,5.803523650718598 +200000,1,21.800211,0.5097258749883622,0.10255850001703948,0.08821750001516193,0.5198811669833958,4.970098771956242,5.778058490670849 diff --git a/benchmarks/write_speedup.png b/benchmarks/write_speedup.png index 5ff73f5..d55ecf3 100644 Binary files a/benchmarks/write_speedup.png and b/benchmarks/write_speedup.png differ diff --git a/libextxyz/_extxyz.def b/libextxyz/_extxyz.def index 0559cd6..591b3fe 100644 --- a/libextxyz/_extxyz.def +++ b/libextxyz/_extxyz.def @@ -7,6 +7,8 @@ EXPORTS extxyz_write_ll_fmt print_dict free_dict + extxyz_dispatch_init + extxyz_dispatch_free extxyz_fopen extxyz_fclose extxyz_ftell diff --git a/libextxyz/_extxyz_pyext.def b/libextxyz/_extxyz_pyext.def index 1cb1b02..b1ff0bc 100644 --- a/libextxyz/_extxyz_pyext.def +++ b/libextxyz/_extxyz_pyext.def @@ -12,6 +12,8 @@ EXPORTS extxyz_write_ll_fmt print_dict free_dict + extxyz_dispatch_init + extxyz_dispatch_free extxyz_fopen extxyz_fclose extxyz_ftell diff --git a/libextxyz/extxyz.c b/libextxyz/extxyz.c index b95f384..0771bdf 100644 --- a/libextxyz/extxyz.c +++ b/libextxyz/extxyz.c @@ -8,6 +8,7 @@ #include "extxyz_kv_grammar.h" #include "extxyz.h" +#include "extxyz_dispatch.h" #include "fast_format.h" void init_DictEntry(DictEntry *entry, const char *key, const int key_len) { @@ -415,9 +416,15 @@ void free_DataLinkedList(DataLinkedList *list, enum data_type data_t, int free_s return; } + (void) data_t; DataLinkedList *next_data; for (DataLinkedList *data = list; data; data = next_data) { - if (data_t == data_s && free_string_content) { + // Key string-freeing on each node's own type, not the entry's: on a + // parse error the list is freed before DataLinkedList_to_data sets the + // entry data_t, so an entry data_t of data_none would otherwise leak + // the per-node string content. (On success the list is already NULL + // here, so this loop is a no-op and never double-frees.) + if (data->data_t == data_s && free_string_content) { free(data->data.s); } next_data = data->next; @@ -688,7 +695,7 @@ char *read_line(char **line, unsigned long *line_len, FILE *fp) { // validating each field, instead of compiling and matching a per-line PCRE2 // regex. Faster, opt-in; slightly more lenient than the grammar on numeric // edge cases. extxyz_read_ll (below) is the regex default. -int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message, int use_tokenizer) { +int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message, int use_tokenizer, int use_cleri) { char *line; unsigned long line_len; unsigned long line_len_init = 1024; @@ -724,24 +731,34 @@ int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEnt return 0; } // actually parse - optionally replace line read from file with `comment` argument - cleri_parse_t * tree; - if (comment != NULL) { - tree = cleri_parse(kv_grammar, comment); - } else { - tree = cleri_parse(kv_grammar, line); - } - if (! tree->is_valid) { - sprintf(error_message, "Failed to parse string at pos %zd", tree->pos); + // use_cleri (default) walks the libcleri grammar; otherwise the equivalent + // first-char-dispatch parser (extxyz_dispatch_parse) builds the same dict. + if (use_cleri) { + cleri_parse_t * tree; + if (comment != NULL) { + tree = cleri_parse(kv_grammar, comment); + } else { + tree = cleri_parse(kv_grammar, line); + } + if (! tree->is_valid) { + sprintf(error_message, "Failed to parse string at pos %zd", tree->pos); + cleri_parse_free(tree); + free(line); + return 0; + } + *info = tree_to_dict(tree, error_message); cleri_parse_free(tree); - free(line); - return 0; - } - *info = tree_to_dict(tree, error_message); - cleri_parse_free(tree); - if (! info) { - sprintf(error_message, "Failed to convert tree to dict"); - free(line); - return 0; + if (! *info) { + sprintf(error_message, "Failed to convert tree to dict"); + free(line); + return 0; + } + } else { + *info = extxyz_dispatch_parse(comment != NULL ? comment : line, error_message); + if (! *info) { + free(line); + return 0; + } } // grab and parse Properties string @@ -1096,9 +1113,10 @@ int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEnt return 1; } -// Backward-compatible reader: per-atom lines parsed with the PCRE2 regex. +// Backward-compatible reader: per-atom lines parsed with the PCRE2 regex, +// comment line parsed with the libcleri grammar. int extxyz_read_ll(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message) { - return extxyz_read_ll_opts(kv_grammar, fp, nat, info, arrays, comment, error_message, 0); + return extxyz_read_ll_opts(kv_grammar, fp, nat, info, arrays, comment, error_message, 0, 1); } //////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/libextxyz/extxyz.h b/libextxyz/extxyz.h index e474933..04b5961 100644 --- a/libextxyz/extxyz.h +++ b/libextxyz/extxyz.h @@ -69,7 +69,7 @@ typedef struct dict_entry_struct { void print_dict(DictEntry *dict); void free_dict(DictEntry *dict); int extxyz_read_ll(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message); -int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message, int use_tokenizer); +int extxyz_read_ll_opts(cleri_grammar_t *kv_grammar, FILE *fp, int *nat, DictEntry **info, DictEntry **arrays, char *comment, char *error_message, int use_tokenizer, int use_cleri); int extxyz_write_ll(FILE *fp, int nat, DictEntry *info, DictEntry *arrays); int extxyz_write_ll_fmt(FILE *fp, int nat, DictEntry *info, DictEntry *arrays, const char *fmt_i, const char *fmt_f, diff --git a/libextxyz/extxyz_dispatch.c b/libextxyz/extxyz_dispatch.c new file mode 100644 index 0000000..145e26e --- /dev/null +++ b/libextxyz/extxyz_dispatch.c @@ -0,0 +1,358 @@ +/* First-char-dispatch parser for the extended-XYZ comment line. + * + * Grammar-faithful: reproduces the pyleri/libcleri accepted language and + * produces a DictEntry list identical to tree_to_dict(), but dispatches each + * value on its first non-space character so only the relevant val_item + * alternative(s) are tried instead of libcleri walking the whole ordered + * Choice, and folds parse + marshalling into one pass. + * + * Token extents are validated with the EXACT grammar PCRE2 patterns (the same + * INTEGER_RE/FLOAT_RE/BOOL_RE the generated grammar uses), so the accepted + * language matches by construction. Output parity is then guaranteed by reusing + * the real finalize (DataLinkedList_to_data) + helpers from extxyz.c. + */ +#include +#include +#include +#include + +#define PCRE2_CODE_UNIT_WIDTH 8 +#include + +#include /* for cleri_grammar_t referenced in extxyz.h */ +#include "extxyz.h" +#include "extxyz_kv_grammar.h" /* INTEGER_RE, FLOAT_RE, BOOL_RE */ +#include "extxyz_dispatch.h" + +/* ---- reused from extxyz.c (non-static) ---- */ +extern void init_DictEntry(DictEntry *entry, const char *key, const int key_len); +extern int DataLinkedList_to_data(DictEntry *dict, char *error_message); +extern double atof_eEdD(char *str); +extern void unquote(char *str); + +/* ---- regex strings the generated grammar uses (from grammar/extxyz_kv_grammar.py) ---- */ +#define BARESTRING_RE "(?:[^\\s=\",}{\\]\\[\\\\]|(?:\\\\[\\s=\",}{\\]\\[\\\\]))+" +#define DQ_RE "(\")(?:(?=(\\\\?))\\2.)*?\\1" +#define CB_RE "{(?:[^{}]|\\\\[{}])*(?= 0) { + PCRE2_SIZE *ov = pcre2_get_ovector_pointer(md); + out = (int)(ov[1] - ov[0]); + } + pcre2_match_data_free(md); + return out; +} + +/* ---- DataLinkedList append (mirrors parse_tree) ---- */ +static void append_item(DictEntry *e, enum data_type t, int iv, double fv, int bv, char *sv) { + DataLinkedList *d = (DataLinkedList *)malloc(sizeof(DataLinkedList)); + d->next = 0; d->data_t = t; + if (t == data_i) d->data.i = iv; + else if (t == data_f) d->data.f = fv; + else if (t == data_b) d->data.b = bv; + else d->data.s = sv; + if (!e->first_data_ll) e->first_data_ll = d; else e->last_data_ll->next = d; + e->last_data_ll = d; +} + +/* dup [pos,pos+n) into a fresh NUL-terminated buffer */ +static char *dupn(const char *s, size_t pos, int n) { + char *r = (char *)malloc(n + 1); + memcpy(r, s + pos, n); r[n] = 0; return r; +} + +static int is_ws(char c) { return c==' '||c=='\t'||c=='\n'||c=='\r'||c=='\f'||c=='\v'; } +static void skip_ws(const char *s, size_t len, size_t *pos) { while (*pos=len || s[p]==close) return 0; /* trailing comma */ + } else if (require_comma) { + return 0; /* bracket array needs a comma */ + } + /* else: whitespace already skipped by caller -> whitespace separator */ + *pp = p; + return 1; +} + +/* Try to parse a container body (after the opening quote/bracket) of numbers/ + bools. Returns body element count and sets *type, or -1 if it isn't a clean + numeric/bool list. require_comma distinguishes "[...]" (commas) from old + "..."/{...} containers (whitespace or commas). */ +static int parse_old_body(const char *s, size_t len, size_t *pp, char close, + DictEntry *e, enum data_type *type, int require_comma) { + size_t p = *pp; int count = 0; enum data_type t = data_none; + for (;;) { + skip_ws(s, len, &p); + if (p=len) return -1; + if (count>0 && !consume_sep(s, len, &p, close, require_comma)) return -1; + /* dispatch the element on its first char to avoid trying every type */ + char ec=s[p]; enum data_type et; int n; + if ((ec>='0'&&ec<='9')||ec=='+'||ec=='-'||ec=='.') { + int ni=rmatch(RX_INT,s,len,p), nf=rmatch(RX_FLOAT,s,len,p); + if (ni>0 && ni>=nf) { et=data_i; n=ni; } + else if (nf>0) { et=data_f; n=nf; } + else return -1; + } else { + int nb=rmatch(RX_BOOL,s,len,p); + if (nb>0) { et=data_b; n=nb; } + else return -1; /* not numeric/bool -> caller falls back to string */ + } + /* homogeneity with promotion: i/f mix -> f; anything else must match */ + if (t==data_none) t=et; + else if ((t==data_i||t==data_f)&&(et==data_i||et==data_f)) { if (et==data_f||t==data_f) t=data_f; } + else if (t!=et) return -1; + store_scalar(e, et==data_i?RX_INT:et==data_f?RX_FLOAT:RX_BOOL, s, p, n); + p += n; count++; + } + if (p>=len || s[p]!=close) return -1; + p++; /* consume close */ + *pp = p; *type = t; + return count; +} + +/* parse a list of r_string elements until `close` (one_d_array_s / strings_sp). + Returns count, or -1 if an element isn't a valid string or close is missing. + require_comma as in parse_old_body. */ +static int parse_string_list(const char *s, size_t len, size_t *pp, char close, + DictEntry *e, int require_comma) { + size_t p=*pp; int count=0; + for (;;) { + skip_ws(s,len,&p); + if (p=len) return -1; + if (count>0 && !consume_sep(s, len, &p, close, require_comma)) return -1; + char c=s[p]; int n; int which=-1; + if (c=='"') which=RX_DQ; else if (c=='{') which=RX_CB; else if (c=='[') which=RX_SB; + if (which>=0) { n=rmatch(which,s,len,p); } + else { n=rmatch(RX_BARE,s,len,p); } + if (n<=0) return -1; + char *tok=dupn(s,p,n); if (which>=0) unquote(tok); + append_item(e,data_s,0,0,0,tok); + p+=n; count++; + } + if (p>=len || s[p]!=close) return -1; + p++; *pp=p; return count; +} + +/* free any partial data list on an entry (used on old-container backtrack) */ +static void reset_entry_data(DictEntry *e) { + DataLinkedList *d=e->first_data_ll, *nx; + while (d){ nx=d->next; if (d->data_t==data_s) free(d->data.s); free(d); d=nx; } + e->first_data_ll=e->last_data_ll=0; e->n_in_row=0; e->nrows=e->ncols=0; +} + +/* parse one value into entry e; returns end pos or (size_t)-1 on failure */ +static size_t parse_value(const char *s, size_t len, size_t pos, DictEntry *e) { + skip_ws(s, len, &pos); + if (pos>=len) return (size_t)-1; + char c = s[pos]; + + /* --- new-style bracket array [...] / [[...]] (comma-delimited) --- */ + if (c=='[') { + size_t probe=pos+1; skip_ws(s,len,&probe); + int two_d = (probe=len) return (size_t)-1; + if (nrows>0 && !consume_sep(s,len,&p,']',1)) return (size_t)-1; + if (p>=len || s[p]!='[') return (size_t)-1; + size_t rs=p+1, rp=p+1; enum data_type t; + int n=parse_old_body(s,len,&rp,']',e,&t,1); + if (n<0) { /* string row (one_d_array_s) */ + rp=rs; n=parse_string_list(s,len,&rp,']',e,1); + if (n<0) return (size_t)-1; + } + p=rp; + if (ncols<0) ncols=n; else if (ncols!=n) return (size_t)-1; + nrows++; + } + e->nrows=nrows; e->ncols=ncols; + } else { + enum data_type t; int n=parse_old_body(s,len,&p,']',e,&t,1); + if (n<0) { /* one_d_array_s: all-string [...]; else sb-string scalar */ + reset_entry_data(e); + size_t ps=pos+1; int sc=parse_string_list(s,len,&ps,']',e,1); + if (sc>0) { e->nrows=0; e->ncols=sc; return ps; } + reset_entry_data(e); + int sl=rmatch(RX_SB,s,len,pos); + if (sl<0) return (size_t)-1; + char *tok=dupn(s,pos,sl); unquote(tok); + append_item(e,data_s,0,0,0,tok); e->nrows=e->ncols=0; + return pos+sl; + } + e->nrows=0; e->ncols=n; + } + return p; + } + + /* --- old-style container "..." {...} OR quoted string --- */ + if (c=='"' || c=='\'' || c=='{') { + char close = (c=='{') ? '}' : c; + size_t p=pos+1; enum data_type t; + int n=parse_old_body(s,len,&p,close,e,&t,0); + if (n>=0) { + /* old-array shape rules */ + if (n==1) { e->nrows=0; e->ncols=0; } /* single -> scalar */ + else if (n==9) { e->nrows=-3; e->ncols=-3; } /* 9 -> 3x3 transpose */ + else { e->nrows=0; e->ncols=n; } + return p; + } + /* {…} also allows a string list (old_one_d_array strings branch) */ + if (c=='{') { + reset_entry_data(e); + size_t ps=pos+1; int sc=parse_string_list(s,len,&ps,'}',e,0); + if (sc>0) { + if (sc==1){e->nrows=0;e->ncols=0;} else if (sc==9){e->nrows=-3;e->ncols=-3;} + else {e->nrows=0;e->ncols=sc;} + return ps; + } + reset_entry_data(e); + } + /* backtrack -> quoted string. Single quotes are not a string container. */ + reset_entry_data(e); + if (c=='\'') return (size_t)-1; + int which = (c=='"') ? RX_DQ : RX_CB; + int sl=rmatch(which,s,len,pos); + if (sl<0) return (size_t)-1; + char *tok=dupn(s,pos,sl); unquote(tok); + append_item(e,data_s,0,0,0,tok); e->nrows=e->ncols=0; + return pos+sl; + } + + /* --- scalar: most_greedy over {int,float,bool,bare-string} --- */ + int ni=rmatch(RX_INT,s,len,pos), nf=rmatch(RX_FLOAT,s,len,pos); + int nb=rmatch(RX_BOOL,s,len,pos), ns=rmatch(RX_BARE,s,len,pos); + int best=-1, which=-1; + if (ni>best){best=ni;which=RX_INT;} + if (nf>best){best=nf;which=RX_FLOAT;} + if (nb>best){best=nb;which=RX_BOOL;} + if (ns>best){best=ns;which=RX_BARE;} + if (best<=0) return (size_t)-1; + store_scalar(e, which, s, pos, best); + e->nrows=0; e->ncols=0; + return pos+best; +} + +/* parse a key into *keyout (NUL-terminated, unquoted); returns end pos or -1 */ +static size_t parse_key(const char *s, size_t len, size_t pos, char **keyout, int *keylen) { + skip_ws(s,len,&pos); + if (pos>=len) return (size_t)-1; + char c=s[pos]; + if (c=='"'||c=='{'||c=='[') { + int which=(c=='"')?RX_DQ:(c=='{')?RX_CB:RX_SB; + int n=rmatch(which,s,len,pos); if (n<0) return (size_t)-1; + char *k=dupn(s,pos,n); unquote(k); *keyout=k; *keylen=(int)strlen(k); + return pos+n; + } + int n=rmatch(RX_BARE,s,len,pos); if (n<0) return (size_t)-1; + *keyout=dupn(s,pos,n); *keylen=n; + return pos+n; +} + +static void set_parse_error(char *error_message, size_t pos) { + if (error_message) sprintf(error_message, "Failed to parse string at pos %zd", pos); +} + +/* Public: parse a comment line, returns DictEntry head or NULL on reject. */ +DictEntry *extxyz_dispatch_parse(const char *s, char *error_message) { + extxyz_dispatch_init(); /* idempotent; no-op once compiled */ + size_t len=strlen(s), pos=0; + DictEntry *dict=(DictEntry*)malloc(sizeof(DictEntry)); + init_DictEntry(dict,0,-1); + DictEntry *cur=dict; + + skip_ws(s,len,&pos); + while (pos first (all_kv_pair order) */ + int handled=0; + if ((len-pos)>=10 && strncasecmp(s+pos,"properties",10)==0) { + size_t p=pos+10; + if (p>=len || !(isalnum((unsigned char)s[p])||s[p]=='_')) { /* keyword \b */ + skip_ws(s,len,&p); + if (p0) { + if (cur->key){ DictEntry*ne=malloc(sizeof(DictEntry)); cur->next=ne; cur=ne; } + init_DictEntry(cur,"Properties",10); + char *v=dupn(s,p,pl); append_item(cur,data_s,0,0,0,v); + cur->nrows=cur->ncols=0; + pos=p+pl; handled=1; + } + } + } + } + if (!handled) { + char *key; int klen; + size_t kp=parse_key(s,len,pos,&key,&klen); + if (kp==(size_t)-1){ free_dict(dict); set_parse_error(error_message,pos); return NULL; } + skip_ws(s,len,&kp); + if (kp>=len || s[kp]!='='){ free(key); free_dict(dict); set_parse_error(error_message,kp); return NULL; } + kp++; + if (cur->key){ DictEntry*ne=malloc(sizeof(DictEntry)); cur->next=ne; cur=ne; } + init_DictEntry(cur,key,klen); free(key); + size_t vp=parse_value(s,len,kp,cur); + if (vp==(size_t)-1){ free_dict(dict); set_parse_error(error_message,kp); return NULL; } + pos=vp; + } + skip_ws(s,len,&pos); + } + + char err[1024]; + if (DataLinkedList_to_data(dict,err)) { + free_dict(dict); + if (error_message) sprintf(error_message, "Failed to parse string (tree to dict)"); + return NULL; + } + return dict; +} diff --git a/libextxyz/extxyz_dispatch.h b/libextxyz/extxyz_dispatch.h new file mode 100644 index 0000000..1dcaed1 --- /dev/null +++ b/libextxyz/extxyz_dispatch.h @@ -0,0 +1,33 @@ +/* First-char-dispatch parser for the extended-XYZ comment line. + * + * A hand-written, grammar-faithful alternative to the libcleri grammar walk + * (cleri_parse + tree_to_dict): it dispatches each value on its first non-space + * character so only the relevant val_item alternative(s) are tried, and folds + * parse + marshalling into a single pass. It accepts EXACTLY the same language + * as the pyleri/libcleri grammar and produces a byte-identical DictEntry list; + * provability is maintained by a differential conformance test against cleri + * (tests/test_dispatch_parity.py), with cleri retained as the canonical grammar. + * + * Selected at read time by use_cleri=0 (see extxyz_read_ll_opts). Reuses the + * grammar's own PCRE2 token patterns (INTEGER_RE/FLOAT_RE/BOOL_RE/...), so the + * accepted token language is identical by construction. + */ +#ifndef EXTXYZ_DISPATCH_H +#define EXTXYZ_DISPATCH_H + +struct dict_entry_struct; + +/* Compile + JIT the token regexes once. Idempotent; safe to call repeatedly. + * Called eagerly at Python import and lazily on first parse otherwise. */ +void extxyz_dispatch_init(void); + +/* Free the cached compiled regexes (process-exit cleanup, mirrors the grammar + * free); leaves the parser re-initialisable. */ +void extxyz_dispatch_free(void); + +/* Parse comment line `s`. Returns a DictEntry linked list identical to + * tree_to_dict(), or NULL on a parse error (with `error_message`, if non-NULL, + * set to a "Failed to parse string ..." message matching the cleri path). */ +struct dict_entry_struct *extxyz_dispatch_parse(const char *s, char *error_message); + +#endif /* EXTXYZ_DISPATCH_H */ diff --git a/libextxyz/meson.build b/libextxyz/meson.build index b60ce0b..cb4cf1e 100644 --- a/libextxyz/meson.build +++ b/libextxyz/meson.build @@ -8,7 +8,7 @@ run_command( ) # Build and install the extension module -extxyz_c_sources = ['extxyz.c', 'extxyz_kv_grammar.c', 'fast_format.c'] +extxyz_c_sources = ['extxyz.c', 'extxyz_kv_grammar.c', 'fast_format.c', 'extxyz_dispatch.c'] # The _extxyz extension is loaded both via ctypes.CDLL (for write/grammar/stdio) # and — when built with numpy — imported as a real C-API module for the fast diff --git a/libextxyz/pyext.c b/libextxyz/pyext.c index ce24aff..f234200 100644 --- a/libextxyz/pyext.c +++ b/libextxyz/pyext.c @@ -166,7 +166,8 @@ static PyObject *dict_to_py(DictEntry *head) } /* read_frame(grammar_addr:int, fp_addr:int, use_tokenizer:int, - * comment:str|None=None) -> (nat:int, info:dict, arrays:dict) + * comment:str|None=None, use_cleri:int=1) + * -> (nat:int, info:dict, arrays:dict) * Raises EOFError at end of file, ExtXYZError on a parse error. */ static PyObject *py_read_frame(PyObject *self, PyObject *args) { @@ -174,8 +175,9 @@ static PyObject *py_read_frame(PyObject *self, PyObject *args) unsigned long long grammar_addr, fp_addr; int use_tokenizer; const char *comment = NULL; - if (!PyArg_ParseTuple(args, "KKi|z", &grammar_addr, &fp_addr, - &use_tokenizer, &comment)) + int use_cleri = 1; + if (!PyArg_ParseTuple(args, "KKi|zi", &grammar_addr, &fp_addr, + &use_tokenizer, &comment, &use_cleri)) return NULL; cleri_grammar_t *grammar = (cleri_grammar_t *)(uintptr_t)grammar_addr; @@ -189,7 +191,8 @@ static PyObject *py_read_frame(PyObject *self, PyObject *args) int ok; Py_BEGIN_ALLOW_THREADS ok = extxyz_read_ll_opts(grammar, fp, &nat, &info, &arrays, - (char *)comment, error_message, use_tokenizer); + (char *)comment, error_message, use_tokenizer, + use_cleri); Py_END_ALLOW_THREADS if (!ok) { diff --git a/libextxyz/test_C_main.c b/libextxyz/test_C_main.c index a4bf636..87056c3 100644 --- a/libextxyz/test_C_main.c +++ b/libextxyz/test_C_main.c @@ -5,8 +5,8 @@ #include "extxyz.h" int main(int argc, char *argv[]) { - if (argc < 3 || argc > 4) { - fprintf(stderr, "Usage: %s filename verbose [tok]\n", argv[0]); + if (argc < 3 || argc > 5) { + fprintf(stderr, "Usage: %s filename verbose [tok] [disp]\n", argv[0]); exit(1); } FILE *fp = fopen(argv[1], "r"); @@ -26,8 +26,10 @@ int main(int argc, char *argv[]) { int verbose = ! strcmp(argv[2], "T"); // optional 3rd arg "tok"/"T" selects the whitespace tokenizer over the regex - int use_tokenizer = (argc == 4 && (! strcmp(argv[3], "tok") || ! strcmp(argv[3], "T"))); - int success = extxyz_read_ll_opts(kv_grammar, fp, &nat, &info, &arrays, comment, error_message, use_tokenizer); + int use_tokenizer = (argc >= 4 && (! strcmp(argv[3], "tok") || ! strcmp(argv[3], "T"))); + // optional 4th arg "disp" selects the first-char-dispatch comment parser + int use_cleri = ! (argc == 5 && ! strcmp(argv[4], "disp")); + int success = extxyz_read_ll_opts(kv_grammar, fp, &nat, &info, &arrays, comment, error_message, use_tokenizer, use_cleri); if (! success) { fprintf(stderr, "ERROR parsing '%s': %s\n", argv[1], error_message); } else { diff --git a/pyproject.toml b/pyproject.toml index 282be24..fb05cf3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,7 @@ classifiers = [ "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", "Programming Language :: C", "Programming Language :: Fortran", ] diff --git a/python/ase-extxyz/ase_extxyz/io.py b/python/ase-extxyz/ase_extxyz/io.py index e08c3aa..f2970c1 100644 --- a/python/ase-extxyz/ase_extxyz/io.py +++ b/python/ase-extxyz/ase_extxyz/io.py @@ -290,6 +290,7 @@ def _normalize_index(index): def read_cextxyz(filename, index=-1, *, use_cextxyz: bool = True, use_regex: bool = False, + use_cleri: bool = True, create_calc: bool = False, calc_prefix: str = '', verbose: int = 0): @@ -298,6 +299,9 @@ def read_cextxyz(filename, index=-1, *, Generator that yields one ``Atoms`` per frame, sliced by ``index``. ``filename`` is a path-like — ASE passes the path through to us because the format's IOFormat code is ``+S`` (see :data:`cextxyz_format`). + + ``use_cleri`` (C backend only): True (default) parses the comment line with + the libcleri grammar; False uses the faster first-char dispatch parser. """ forward, post = _normalize_index(index) @@ -305,6 +309,7 @@ def read_cextxyz(filename, index=-1, *, for frame in extxyz.iread_dicts(filename, index=forward, use_cextxyz=use_cextxyz, use_regex=use_regex, + use_cleri=use_cleri, verbose=verbose): yield _frame_to_atoms(frame, create_calc=create_calc, calc_prefix=calc_prefix) @@ -314,6 +319,7 @@ def read_cextxyz(filename, index=-1, *, frames = list(extxyz.iread_dicts(filename, use_cextxyz=use_cextxyz, use_regex=use_regex, + use_cleri=use_cleri, verbose=verbose)) if isinstance(post, int): yield _frame_to_atoms(frames[post], create_calc=create_calc, diff --git a/python/ase-extxyz/tests/conftest.py b/python/ase-extxyz/tests/conftest.py index e3211d3..b8db860 100644 --- a/python/ase-extxyz/tests/conftest.py +++ b/python/ase-extxyz/tests/conftest.py @@ -30,13 +30,21 @@ def write(file, atoms, **kwargs): verbose = 0 +# USE_CLERI=false runs the C backend through the first-char dispatch comment +# parser instead of the libcleri grammar (default True). Ignored by the +# pure-Python backend (use_cextxyz=False), which always uses pyleri. +_use_cleri = not os.environ.get('USE_CLERI', '').lower().startswith('f') + if 'USE_CEXTXYZ' in os.environ: _flag = os.environ['USE_CEXTXYZ'].lower().startswith('t') - read_kwargs_variants = [{'use_regex': False, 'use_cextxyz': _flag}] + read_kwargs_variants = [{'use_regex': False, 'use_cextxyz': _flag, + 'use_cleri': _use_cleri}] write_kwargs_variants = [{'use_cextxyz': _flag}] else: - read_kwargs_variants = [{'use_regex': False, 'use_cextxyz': False}, - {'use_regex': False, 'use_cextxyz': True}] + read_kwargs_variants = [{'use_regex': False, 'use_cextxyz': False, + 'use_cleri': _use_cleri}, + {'use_regex': False, 'use_cextxyz': True, + 'use_cleri': _use_cleri}] write_kwargs_variants = [{'use_cextxyz': False}, {'use_cextxyz': True}] diff --git a/python/extxyz/cextxyz.py b/python/extxyz/cextxyz.py index b41814a..d315dc8 100644 --- a/python/extxyz/cextxyz.py +++ b/python/extxyz/cextxyz.py @@ -244,6 +244,14 @@ def py_to_c_dict(py_dict, keys=None): # construct grammar only once on module initialisation _kv_grammar = extxyz.compile_extxyz_kv_grammar() +# Pre-compile the first-char dispatcher's token regexes once, here at import +# (single-threaded), so the lazy in-C init never races between concurrent +# reads. Guarded by hasattr: a build whose _extxyz didn't export it (the symbol +# is in _extxyz.def) still works via the C function's own lazy init. +_have_dispatch = hasattr(extxyz, 'extxyz_dispatch_init') +if _have_dispatch: + extxyz.extxyz_dispatch_init() + @atexit.register def _free_kv_grammar(): @@ -258,6 +266,8 @@ def _free_kv_grammar(): if _have_grammar_free: extxyz.cleri_grammar_free(_kv_grammar) _kv_grammar = None + if _have_dispatch: + extxyz.extxyz_dispatch_free() # On Windows, route stdio through wrappers in _extxyz so we use the same # C runtime as extxyz_read_ll/extxyz_write_ll. find_library('c') returns @@ -300,7 +310,8 @@ def cfseek(fp, offset, whence): return _fseek(fp, offset, whence) -def read_frame_dicts(fp, verbose=False, comment=None, use_regex=False): +def read_frame_dicts(fp, verbose=False, comment=None, use_regex=False, + use_cleri=True): """Read a single frame, returning ``(nat, info, arrays)``. Uses the C-API ``_extxyz.read_frame`` fast path (read + dict marshalling in @@ -317,6 +328,10 @@ def read_frame_dicts(fp, verbose=False, comment=None, use_regex=False): with the fast whitespace tokenizer, which validates each field. If True, use the slower PCRE2 regex parser instead (marginally stricter than the tokenizer on numeric edge cases). + use_cleri (bool, optional): if True (default), parse the comment line + with the libcleri grammar. If False, use the faster first-char + dispatch parser, which accepts the same language (validated by a + differential conformance test) and builds the same dicts. Returns: nat, info, arrays: int, dict, dict @@ -324,7 +339,8 @@ def read_frame_dicts(fp, verbose=False, comment=None, use_regex=False): if _HAVE_C_READ and not _USE_LEGACY_MARSHAL and not verbose: try: return _ext_mod.read_frame(_kv_grammar.value, fp.value, - 0 if use_regex else 1, comment) + 0 if use_regex else 1, comment, + 1 if use_cleri else 0) except _ext_mod.ExtXYZError as exc: # Re-raise as the canonical cextxyz.ExtXYZError so callers (and # tests) catch one exception type regardless of backend. Normalise @@ -332,10 +348,11 @@ def read_frame_dicts(fp, verbose=False, comment=None, use_regex=False): # core.py's "Failed to parse string" fallback still matches. raise ExtXYZError(str(exc).strip().replace('\n', '')) from None return read_frame_dicts_ctypes(fp, verbose=verbose, comment=comment, - use_regex=use_regex) + use_regex=use_regex, use_cleri=use_cleri) -def read_frame_dicts_ctypes(fp, verbose=False, comment=None, use_regex=False): +def read_frame_dicts_ctypes(fp, verbose=False, comment=None, use_regex=False, + use_cleri=True): """Read a single frame using extxyz_read_ll_opts() and marshal the C dictionaries to Python via ctypes (the original, slower path). @@ -361,7 +378,8 @@ def read_frame_dicts_ctypes(fp, verbose=False, comment=None, use_regex=False): ctypes.byref(arrays), comment, error_message, - ctypes.c_int(0 if use_regex else 1)): + ctypes.c_int(0 if use_regex else 1), + ctypes.c_int(1 if use_cleri else 0)): failure = True if (error_message.value == b'' or error_message.value.decode().startswith("Failed to parse int natoms from ' ")): diff --git a/python/extxyz/core.py b/python/extxyz/core.py index c6598b6..ed45de5 100644 --- a/python/extxyz/core.py +++ b/python/extxyz/core.py @@ -117,15 +117,16 @@ def _read_frame_pure_python(file, verbose=0, use_regex=False): return natoms, info, data, properties -def _read_frame_dict(file, *, use_cextxyz=True, use_regex=False, verbose=0, - comment=None) -> Frame | None: +def _read_frame_dict(file, *, use_cextxyz=True, use_regex=False, use_cleri=True, + verbose=0, comment=None) -> Frame | None: """Read one frame and return a :class:`Frame`, or ``None`` past EOF.""" try: if use_cextxyz: try: fpos = cextxyz.cftell(file) natoms, info, arrays = cextxyz.read_frame_dicts( - file, verbose=verbose, comment=comment, use_regex=use_regex) + file, verbose=verbose, comment=comment, use_regex=use_regex, + use_cleri=use_cleri) except cextxyz.ExtXYZError as msg: error_message, = msg.args if error_message.startswith('Failed to parse string'): @@ -133,7 +134,7 @@ def _read_frame_dict(file, *, use_cextxyz=True, use_regex=False, verbose=0, natoms, info, arrays = cextxyz.read_frame_dicts( file, verbose=verbose, comment="Properties=species:S:1:pos:R:3", - use_regex=use_regex) + use_regex=use_regex, use_cleri=use_cleri) else: raise info.pop('Properties', None) @@ -162,7 +163,8 @@ def _read_frame_dict(file, *, use_cextxyz=True, use_regex=False, verbose=0, def iread_dicts(file, index=None, *, - use_cextxyz=True, use_regex=False, verbose=0, comment=None + use_cextxyz=True, use_regex=False, use_cleri=True, verbose=0, + comment=None ) -> Iterator[Frame]: """Yield :class:`Frame` instances from ``file`` lazily. @@ -200,8 +202,8 @@ def iread_dicts(file, index=None, *, for frame_idx in frame_indices: while current_frame <= frame_idx: f = _read_frame_dict(file, use_cextxyz=use_cextxyz, - use_regex=use_regex, verbose=verbose, - comment=comment) + use_regex=use_regex, use_cleri=use_cleri, + verbose=verbose, comment=comment) current_frame += 1 if f is None: break diff --git a/tests/test_dispatch_parity.py b/tests/test_dispatch_parity.py new file mode 100644 index 0000000..699d93b --- /dev/null +++ b/tests/test_dispatch_parity.py @@ -0,0 +1,165 @@ +"""Differential conformance: the first-char dispatch comment-line parser +(use_cleri=False) must accept the same language and produce byte-identical +results to the libcleri grammar (use_cleri=True). This test IS the provability +mechanism — cleri stays the canonical grammar/oracle. +""" +import random + +import numpy as np +import pytest + +import extxyz +from extxyz import cextxyz + +pytestmark = pytest.mark.skipif( + not cextxyz._HAVE_C_READ, + reason="C read path not built") + +# Comment-line bodies (appended after the mandatory Properties=...) covering +# every value shape: scalars, 1D/2D bracket arrays, old "..."/{...} containers, +# the string-fallback backtracks, scientific floats, quoted/escaped keys. +BODIES = [ + 'energy=-1.5 step=42 ok=T name=astring', + 'q="quoted value" e="esc\\"aped"', + 'iarr="1 2 3" farr="3.3 4.4" barr="T F T"', + 'v=[1, 2, 3] f=[1.5, 2.5] b=[T, F]', + 'm=[[1, 2, 3], [4, 5, 6], [7, 8, 9]]', + 'lat="1 2 3 4 5 6 7 8 9"', + 'cb={1 2 3} cbs={a b c}', + 'sci=1.2e7 sci2=5e-6 sciarr="1.2 2200 0.33"', + 'notnum="1.0 2.0 3.0 7.0 8.09.0"', # -> string (backtrack) + 'notbool="T F S" single="1" mixed="1 2 x"', # -> string / scalar + 'bob#joe=2 "with space"=3 "sp\\"q"=4', + 'pbc="T T F" subset="MC2D"', + 'twostr=[["a", "b"], ["c", "d"]]', +] + +PROPS = "Properties=species:S:1:pos:R:3" +ATOM = "Si 0.0 0.0 0.0" + + +def _frame(body): + return f"1\n{PROPS} {body}\n{ATOM}\n" + + +def _read(path, use_cleri): + out = extxyz.read_dicts(str(path), use_cextxyz=True, use_cleri=use_cleri) + return out if isinstance(out, list) else [out] + + +def _assert_equal(label, a, b): + aa, ba = isinstance(a, np.ndarray), isinstance(b, np.ndarray) + assert aa == ba, f"{label}: array-ness {aa} != {ba} ({a!r} vs {b!r})" + if aa: + assert a.dtype == b.dtype, f"{label}: dtype {a.dtype} != {b.dtype}" + assert a.shape == b.shape, f"{label}: shape {a.shape} != {b.shape}" + if a.dtype.kind == 'f': + assert np.array_equal(a.view('u8'), b.view('u8')), f"{label}: float bits" + else: + assert np.array_equal(a, b), f"{label}: values {a!r} != {b!r}" + else: + assert type(a) is type(b), f"{label}: type {type(a)} != {type(b)}" + assert a == b, f"{label}: {a!r} != {b!r}" + + +def _compare(path): + cl = _read(path, use_cleri=True) + ds = _read(path, use_cleri=False) + assert len(cl) == len(ds) + for i, (a, b) in enumerate(zip(cl, ds)): + assert a.natoms == b.natoms, f"frame {i} natoms" + _assert_equal(f"frame{i}.cell", a.cell, b.cell) + assert (a.pbc == b.pbc).all(), f"frame {i} pbc" + assert set(a.info) == set(b.info), f"frame {i} info keys {set(a.info) ^ set(b.info)}" + for k in a.info: + _assert_equal(f"frame{i}.info[{k!r}]", a.info[k], b.info[k]) + assert set(a.arrays) == set(b.arrays), f"frame {i} arrays keys" + for k in a.arrays: + _assert_equal(f"frame{i}.arrays[{k!r}]", a.arrays[k], b.arrays[k]) + + +@pytest.mark.parametrize("body", BODIES) +def test_dispatch_matches_cleri(tmp_path, body): + path = tmp_path / "f.xyz" + path.write_text(_frame(body)) + _compare(path) + + +def test_dispatch_matches_cleri_multiframe(tmp_path): + path = tmp_path / "multi.xyz" + path.write_text("".join(_frame(b) for b in BODIES)) + _compare(path) + + +# --- randomized differential fuzz over grammar-valid kv lines --- + +def _rand_value(rng): + kind = rng.choice(["int", "float", "bool", "bare", "qstr", + "iarr", "farr", "barr", "old", "m2d"]) + if kind == "int": + return str(rng.randint(-9999, 9999)) + if kind == "float": + return rng.choice([f"{rng.uniform(-1e3, 1e3):.6g}", + f"{rng.uniform(-1, 1):.3e}", f"{rng.randint(0,99)}."]) + if kind == "bool": + return rng.choice(["T", "F", "true", "false", "TRUE", "FALSE"]) + if kind == "bare": + return "".join(rng.choice("abcXYZ_0123") for _ in range(rng.randint(1, 6))) or "x" + if kind == "qstr": + return '"' + rng.choice(["a b", "hello world", "x_1 y_2"]) + '"' + if kind == "iarr": + return "[" + ", ".join(str(rng.randint(-50, 50)) for _ in range(rng.randint(1, 4))) + "]" + if kind == "farr": + return "[" + ", ".join(f"{rng.uniform(-9, 9):.4g}" for _ in range(rng.randint(1, 4))) + "]" + if kind == "barr": + return "[" + ", ".join(rng.choice(["T", "F"]) for _ in range(rng.randint(1, 4))) + "]" + if kind == "old": + return '"' + " ".join(str(rng.randint(-9, 9)) for _ in range(rng.randint(2, 5))) + '"' + # m2d + nc = rng.randint(1, 3) + nr = rng.randint(2, 3) + rows = ["[" + ", ".join(str(rng.randint(-9, 9)) for _ in range(nc)) + "]" for _ in range(nr)] + return "[" + ", ".join(rows) + "]" + + +def test_dispatch_matches_cleri_fuzz(tmp_path): + rng = random.Random(20260616) + path = tmp_path / "fuzz.xyz" + frames = [] + for _ in range(300): + nkv = rng.randint(1, 6) + keys = [f"k{j}{rng.randint(0,99)}" for j in range(nkv)] + body = " ".join(f"{k}={_rand_value(rng)}" for k in keys) + frames.append(_frame(body)) + path.write_text("".join(frames)) + _compare(path) + + +# --- reject parity (at the read_frame_dicts level, bypassing core.py's +# plain-xyz retry fallback) --- + +MALFORMED = [ + "Properties=species:S:1:pos:R:3 bad", # bare key, no '=' + "Properties=species:S:1:pos:R:3 k=1.2.3.4", # invalid value -> reject? compare + "Properties=pos:R:3:species", # malformed Properties + "Properties=species:S:1:pos:R:3 k=[1, 2", # unterminated array + "Properties=species:S:1:pos:R:3 'sq'=1", # single-quoted key not in grammar +] + + +@pytest.mark.parametrize("line", MALFORMED) +def test_dispatch_reject_matches_cleri(tmp_path, line): + """Both backends must agree on accept/reject for the raw comment line.""" + def parse(use_cleri): + p = tmp_path / "m.xyz" + p.write_text(f"1\n{line}\nSi 0 0 0\n") + fp = cextxyz.cfopen(str(p), "r") + try: + cextxyz.read_frame_dicts(fp, use_cleri=use_cleri) + return True # accepted + except cextxyz.ExtXYZError: + return False # rejected + finally: + cextxyz.cfclose(fp) + + assert parse(True) == parse(False), f"accept/reject disagree on {line!r}"