Modernize packaging, add a test suite, fix four methodology defects (v0.3.0) - #4
Merged
Merged
Conversation
Restructures the package and corrects four defects found while reviewing PR #2. Numerical results differ from 0.2.0; every change is in the CHANGELOG with the passage of Kohonen (2013) it follows. Methodology: - Batch training no longer destroys models whose neighbourhood contains no data. It built the new weights from a zeroed array and assigned it wholesale, so untouched models became the zero vector: 282 of 900 on a 30x30 map with 20 samples, in one step. (Eq. 8) - Linear initialization now spans the principal-component plane. It used pca.explained_variance_, two eigenvalues, where it needed pca.components_, the eigenvectors, so every model came out a constant vector: rank 1 rather than a plane. PCA is also fitted on raw data now, so the models share the input space. (Section 4.3) - sequential training honours n_iteration instead of running one pass whatever was asked for. - random training samples i.i.d. with replacement. It previously used replace=(n_iteration > len(data)), making the character of the sampling depend on the iteration count. (Robbins-Monro, cited in Section 4.1) - The automatic map-size ratio uses sqrt(l1/l2), rounds instead of floor-dividing, and clamps to 1. A principal component is a unit direction, so its length in the data is the standard deviation. (Section 3.5) - The neighborhood radius is floored at min_neighborhood_radius, default 0.5. (Section 4.2: sigma "shall not go to zero") - Per-instance np.random.Generator. Constructing a SOM used to reseed NumPy's global generator as a side effect. Seeds therefore do not reproduce 0.1.x/0.2.0 results; pin python-som==0.2.0 for that. Performance: - Batch training is ~30x faster: 1.89s to 0.06s on a 20x20 map with 150 samples over 5 iterations. A test asserts the contracted form matches the literal double loop of Eq. (8) to 1e-12. - distance_matrix no longer materialises an (x,y,x,y) tensor to produce x*y numbers. That cost 800 MB on a 100x100 map; it is now 80 KB, and bit-identical on flat and toroidal grids. Structure and tooling: - src/ layout, split into _som, _neighborhood, _decay and _distance, with py.typed so the annotations are visible downstream. Public imports are unchanged and the pre-0.3.0 private names still resolve. - 190 tests at 100% coverage where there were none, including one regression test per defect carrying its measured numbers, and property-based tests over generated grids, radii and centres. - ruff with select = ALL, mypy --strict, pre-commit, and CI across Python 3.10 to 3.13 that builds the wheel and imports it from a clean environment. Releases publish through PyPI trusted publishing. - Documentation site, and an examples/iris.py that replaces the notebook. - requires-python corrected to >=3.10, which is what PEP 604 annotations in runtime positions actually need.
tomllib entered the stdlib in 3.11, so tests/test_packaging.py failed at collection on 3.10 with ModuleNotFoundError. Local development is on 3.12, so only the CI matrix caught it. Uses tomli where the stdlib lacks tomllib. tomli is the library CPython's tomllib was derived from, first released 2021-05-27, and is pinned as a dev dependency behind a python_version marker so it is not installed where it is not needed.
Every bullet in both files opened with a bolded lead-in: 18 of 18 in the CHANGELOG, 7 of 7 in the README's list of behaviour changes. When everything is emphasised nothing is, and a reader scanning for what affects them gets no help from it. Bold now marks only the one change a reader must not miss, that seeds no longer reproduce results from earlier versions, plus the batch-mode restriction and the instruction to pin 0.2.0. The bullet structure already separates the rest. Content is unchanged: both files are byte-identical once the ** markers are stripped. Marker counts go 24 -> 6 in the README and 38 -> 4 in the CHANGELOG. Found by a pass with the llm-writing-patterns scanner over everything the documentation site ships, including the docstrings that mkdocstrings renders into the API reference. The docstrings needed no changes. The scanner's rule-of-three hits were all genuine enumerations of things that happen to number three, and were left alone.
Three kinds of content were written from the perspective of someone who had just fixed the bugs, rather than of a reader using the library. Internal test names, in four places. `test_batch_never_zeroes_a_model` and friends mean nothing to a reader who has the wheel rather than the repository, and they couple the prose to test filenames. A 30-line "Behavior changes in 0.3.0" section in the README, 20% of the file, three of whose seven bullets restated the CHANGELOG verbatim. The README is the front door for someone deciding whether to use the library; a per-version migration list is reference material that will be stale at 0.4.0 and misleading at 1.0.0, with nobody maintaining it. Replaced with four lines under "Upgrading" that keep the one fact affecting the upgrade decision, that seeds do not reproduce pre-0.3.0 maps, and link to the CHANGELOG for the rest. Every removed fact is still there. An admonition on the training-modes page about `sequential` having once ignored `n_iteration`. The page describes what the modes do; what they used to do belongs in the CHANGELOG. The reproducibility warning stays where seeds are discussed, and the cyclic fold-back explanation stays, because both give a reader context for the current behaviour rather than recounting its history. Also audited for absolute paths, planning-workspace references, PR numbers and TODO markers: none present.
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.
Restructures the package and corrects four methodology defects found while reviewing #2.
Numerical results differ from 0.2.0. Every change is in the CHANGELOG with the
passage of Kohonen (2013) it follows. To reproduce figures made with an earlier version, pin
python-som==0.2.0.Methodology defects fixed
Each was reproduced by running the library, and each has a regression test carrying its measured
numbers.
sequentialignoredn_iterationrandomsampling changed character with the iteration countlen(data), i.i.d. abovex=20→y=6√(λ₁/λ₂)→y=11linear_decaymin_neighborhood_radiusGeneratorPerformance
asserts the contracted form matches the literal double loop of Eq. (8) to
1e-12.distance_matrixno longer builds an(x,y,x,y)tensor to producex*ynumbers. 800 MB → 80 KBon a 100×100 map, bit-identical on flat and toroidal grids.
Structure and tooling
src/layout split into_som,_neighborhood,_decay,_distance, withpy.typed. Publicimports are unchanged and the pre-0.3.0 private names still resolve.
grids, radii and centres.
select = ["ALL"], mypy--strict, pre-commit, CI across Python 3.10–3.13 that builds thewheel and imports it from a clean environment, docs site, and releases through PyPI trusted
publishing.
requires-pythoncorrected to>=3.10, which is what PEP 604 annotations in runtime positionsactually require. The declared
>=3.9could never import.Verification
Notes for review
bubbleneighbourhood is Chebyshev, not Euclidean, so it is not isotropic. The testshipped in 0.2.0 asserted that it was, and passed only because no counterexample exists on a
21×21 grid at σ=3 — the smallest is at radius
√50, needing σ≥5. Behaviour is unchanged andfollows Vrieze's appendix pseudo-code; it is now documented and correctly tested.
learning_rateis still unvalidated.α=5diverges. Left alone here because rejectingpreviously-accepted values belongs in its own change; queued for 0.4.0.
SOMhas 14 instance attributes and a 13-argument constructor. A frozenSOMConfigis planned for0.4.0;
PLR0913/PLR0917are ignored at config level with that justification.Once CI is green this gets tagged
v0.3.0immediately, so master never claims an unpublished version.