Make the project runnable end to end, add tests, docs and a browser demo - #1
Conversation
… artifacts - character_label_classification.ipynb: the activation-maximization cell passed a layer NAME string as seed_input, so tf-keras-vis died on 'Cast string to float is not supported'. seed_input is now a (1,32,32,3) tensor matching the model input. Verified to run; output saved as Images/model2_activation_maximization.png. - Drop unused imports (MobileNet, VGG16, confusion_matrix, ConfusionMatrixDisplay), duplicate numpy/matplotlib imports and commented-out cruft. - Clear the pip log that leaked local Anaconda paths into committed output. - Delete custom_cnn_model_2.zip (redundant copy of custom_cnn_model_2/) and the orphan Images/model2_prediction-2.png. - .gitignore: exclude data/, data2/, *.npz, *.zip, *.rar. - Add SYNC.md documenting local vs origin state.
The notebooks read data/, data2/ and .npz bundles that are not in the repo, so a clone could not run anything. The committed SavedModels plus feature_data/ are enough for inference - the missing piece was the label->index map, which was never saved. It is recoverable: training used LabelBinarizer, which sorts alphabetically, so the class list is just the sorted character column of the committed CSVs. Verified at 46/47 on the sample glyphs. - src/classes.py derive both class lists (47 MODI, 80 combined) from the CSVs - src/model.py SavedModel loading + the notebooks' image preprocessing - src/predict.py CLI: python -m src.predict <image> [--model N] [--topk K] - src/featuremaps.py CLI: render conv activation grids, the repo's namesake - src/export_weights.py flatten a model to float32 + a JSON op plan - requirements.txt pinned to the versions actually verified - docs/DATA.md dataset provenance, expected layout, how to regenerate - tests/ 18 tests over committed artifacts only, no dataset needed - .github/workflows/ci.yml run them on every push and PR
- classes.py: cache the class list as a tuple and hand out a fresh list per call. The sequence order IS the model's output order, so a caller appending to the shared list silently mislabelled every later prediction. Regression test added. - export_weights.py: drop Flatten from _SUPPORTED so an unimplemented layer raises at export time instead of emitting a parameterless stub the browser runtime cannot execute; validate layer activations against what cnn.js implements, for the same reason. - predict.py: reject --topk below 1 at the argparse boundary instead of dying on max() of an empty sequence.
Replaces the single-line placeholder. Title, one-line pitch, what MODI is and why recognising it matters, verified install/usage with real captured output, results tables with the actual trained numbers, the feature-map and activation- maximisation figures, repository layout, a note on how the class mapping was recovered, and an honest limitations section covering the failed multi-label experiment, the 32x32 downsampling, the uncontrolled model comparison and the incomplete local dataset copy. No badges.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:923f6effc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Layer classes the JavaScript runtime knows how to execute. Anything absent | ||
| # here raises at export time, where it is diagnosable, rather than producing a | ||
| # plan the browser cannot run. Keep this in sync with docs/cnn.js. |
There was a problem hiding this comment.
The export explicitly targets docs/cnn.js, but a repo-wide git ls-tree -r/rg check of this commit finds no HTML or JavaScript files at all—docs/ contains only DATA.md and the weight artifacts. Consequently, GitHub Pages has no page to serve and nothing consumes model2.json, model2.bin, or samples.bin, so the advertised browser demo is unusable until its static page and runtime are committed.
Useful? React with 👍 / 👎.
| for _, row in df.iterrows(): | ||
| # Characters shared between the two scripts collapse to one class, so | ||
| # keep the first row seen rather than letting the later one win. | ||
| table.setdefault(str(row["character"]), tuple(int(row[f]) for f in STROKE_FEATURES)) |
There was a problem hiding this comment.
When Model 2 predicts a label shared by both scripts, this always retains the first (MODI) row. This produces incorrect explanatory output for a Devanagari tha: the existing CSV's MODI row at line 36 has strokes o and |, while its Devanagari row at line 81 has o, X, ^, and >. Since predict --model 2 prints this lookup as the predicted character's stroke primitives, ambiguous duplicate labels should be reported as such or handled without silently presenting the MODI vector as valid for both scripts.
Useful? React with 👍 / 👎.
Static page in docs/ - no build step, no CDN, no network calls after load. Model 2's weights are exported to a flat float32 blob plus a JSON op plan and docs/cnn.js runs the forward pass by hand; tests/test_js_parity.py already asserts that plan matches Keras to 1e-4. Design note: drawing is deliberately NOT the primary interaction. MODI glyphs are unfamiliar to any visitor and freehand mouse input is out of distribution, so leading with a blank canvas would both confuse people and understate the model. Instead the 47 real committed glyphs are the gallery, and drawing is a secondary, collapsed panel with a tracing guide and an explicit warning. - Gallery of the 47 real MODI samples, decoded from weights/samples.bin - Top-5 prediction with a match indicator against the true label - Layer-by-layer feature-map explorer across all four conv layers - Occlusion sensitivity sweep, 64 passes, chunked per animation frame - Responsive, light/dark, keyboard accessible, reduced-motion aware The two BatchNorm layers are folded into the following conv at load time - algebraically exact for valid padding - which cuts a forward pass from ~177ms to ~26-65ms and makes the occlusion sweep tolerable at ~4s. Feature maps are genuinely sparse (often 20-35 of 64 channels silent for a given glyph). The UI discloses the per-tile normalisation and the silent-channel count rather than hiding it behind a prettier ramp. Footer links to the repo absolutely: when Pages serves /docs that folder IS the site root, so a relative ../ would leave the repo.
.bar__fill is a <span> and the stylesheet never set display:block, so it stayed display:inline - width and height do not apply to inline boxes, and every bar rendered 0x0. The amber fill and the correct percentage were both there in the DOM, just with no layout box. Verified on the deployed page: the top bar now renders 333px of a 358px track for a 92.9% prediction. The README hero was captured before this fix, so it showed empty bars. Retaken. Also make the footer repo link absolute rather than '../': when Pages serves the /docs folder that folder IS the site root, so the relative link left the repo.
docs/cnn.js implements softmax only in dense(); its conv2d branches on relu and copies everything else through unchanged. The exporter allowed softmax on any layer, so a softmax convolution would have exported cleanly and then silently emitted unnormalised logits in the browser while the UI presented them as probabilities. Conv2D is now restricted to relu/linear, Dense keeps softmax. Exported artifacts are byte-identical; no redeploy needed.
Ssavan99
commented
Aug 16, 2026
Phase 4 landed — the demo is livehttps://ssavan99.github.io/FeatureExtraction_from_AncientScript/ Verified against the deployed URL with a clean browser profile: no console errors, three asset requests ( Two real bugs were caught reviewing the deployed page, not the source:
Note for whoever mergesPages is currently serving |
The repo held solid modelling work — 90.2% test accuracy over 80 MODI + Devanagari classes — behind a one-line README and three notebooks that read
data/,data2/and.npzbundles that are not in the repo. Nothing was runnable from a clone.The unlock
The label→index mapping was never saved with the models, which would normally make the committed weights useless. It turned out to be recoverable: training used
LabelBinarizer, which sorts alphabetically, so the class list is just the sorted character column of the committed CSVs.Verified — Model 1 classifies 46 of the 47 committed sample glyphs correctly (97.9%). That makes the repo fully runnable and testable with no dataset download.
What changed
Hygiene
seed_input, so tf-keras-vis died onCast string to float is not supported. Now a(1,32,32,3)tensor; verified to run, output committed as a figure.custom_cnn_model_2.zip(a redundant copy of the folder beside it) and an orphan image..gitignorenow excludes the datasets.Runnable
src/— class-map recovery, prediction CLI, feature-map CLI, weight export.requirements.txtpinned to versions actually verified.docs/DATA.md— dataset provenance, expected layout, how to regenerate the bundles.Tested
pytest -qexits 0 in ~50s on CPU.tests/test_js_parity.pyre-runs the exported op plan in NumPy and asserts it matches Keras to 1e-4, so the browser demo cannot silently drift from the trained model.Browser demo (
docs/)README — rewritten from one line, with verified commands, real numbers, figures, and honest limitations including the multi-label stroke experiment that failed at ~51% (chance).
Review
/code-reviewat high level found 4 issues; 3 were real code bugs and are fixed in923f6ef:Flattenwas declared exportable but emitted a parameterless stub the JS runtime cannot execute--topk 0crashed onmax()of an empty sequenceThe fourth was a risk flag on whether the TF 2.9.1 pin set resolves on Linux/Python 3.10 — this PR's CI run is what settles it.
Notes
docs/once this merges.