Project hardening: tests, bug fixes, versioning, CI, and docs

.github/workflows/ci.yml

@@ -10,26 +10,34 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
 
-      - uses: actions/setup-python@v5
-        with:
-          python-version: "3.11"
+      - name: Build Docker image
+        run: |
+          docker build \
+            --build-arg UID=$(id -u) \
+            --build-arg GID=$(id -g) \
+            --build-arg UNAME=$(whoami) \
+            -t dtcwt-ci .
 
-      - name: Install dependencies
-        run: pip install -r requirements.txt
+      - name: Lint
+        run: docker run --rm --entrypoint "" dtcwt-ci ruff check .
 
       - name: Verify import
-        run: python -c "import motion_mag"
+        run: docker run --rm --entrypoint "" dtcwt-ci python -c "import motion_mag"
 
       - name: Verify --help
-        run: python motion_mag.py --help
+        run: docker run --rm --entrypoint "" dtcwt-ci python motion_mag.py --help
 
       - name: Verify --version
-        run: python motion_mag.py --version
+        run: docker run --rm --entrypoint "" dtcwt-ci python motion_mag.py --version
 
       - name: Run full pipeline on face.mp4
-        run: python motion_mag.py -i face.mp4 -o face_magnified.avi -k 3 -w 80 --nlevels 4
+        run: |
+          docker run --rm --entrypoint "" \
+            -v "${{ github.workspace }}:/data" \
+            dtcwt-ci \
+            python -u motion_mag.py -i /data/face.mp4 -o /data/face_magnified.avi -k 3 -w 80 --nlevels 4
 
       - name: Verify output exists
         run: test -f face_magnified.avi

.gitignore

@@ -5,6 +5,7 @@
 __pycache__/
 *.py[cod]
 *.egg-info/
+.pytest_cache/
 
 # Output videos
 *.avi

CHANGELOG.md

@@ -0,0 +1,34 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/),
+and this project adheres to [Semantic Versioning](https://semver.org/).
+
+## [Unreleased]
+
+### Added
+- Unit tests for core functions (normalize_phase, flattop_filter, extract_temporal_phases, magnify_motions)
+- Input validation tests for all CLI error paths
+- `VERSION` file as single source of truth for versioning
+- Docker image version labels and tags
+- `CHANGELOG.md`
+- `requirements-dev.txt` for dev dependencies (pytest, ruff)
+- `test.sh` for running lint and tests inside Docker
+- Design doc (`docs/design/dtcwt-hardening.md`)
+- Development section in README (testing, versioning, project structure)
+
+### Fixed
+- `load_video` buffer overflow when `CAP_PROP_FRAME_COUNT` underreports
+- `fps` truncation: keep as float instead of casting to int (3.2% speed error on 29.97fps videos)
+- `flattop_filter_1d` window size guard against zero
+- Ruff F541 lint error (extraneous f-prefix)
+
+### Changed
+- CI modernized: runs inside Docker with ruff linting, updated to actions/checkout v6
+- Dev dependencies (pytest, ruff) baked into Docker image
+- Build script reads version from `VERSION` file, tags image accordingly
+- `dtcwt` dependency pinned with upper bound (`<1`)
+
+### Removed
+- `np.int` monkey-patch — no longer needed with dtcwt 0.14.0

CONTRIBUTING.md

@@ -7,11 +7,12 @@ Thanks for your interest in contributing!
 1. **Open an issue first** — describe the bug or feature you'd like to work on.
 2. **Fork the repo** and create a branch from `main`.
 3. **Keep PRs small** — one logical change per pull request.
-4. **Follow PEP 8** for Python code style.
-5. **Test your changes** — run the CLI on `face.mp4` to verify nothing is broken:
+4. **Follow PEP 8** for Python code style. We use [ruff](https://docs.astral.sh/ruff/) for linting.
+5. **Test your changes** before opening a PR:
    ```bash
-   python motion_mag.py -i face.mp4 -o test_output.avi -k 3 -w 80
+   ./test.sh
    ```
+   Tests run inside Docker — no local Python dependencies needed. See [Development](README.md#development) in the README for details.
 6. **Open a pull request** against `main` with a clear description of your changes.
 
 ## Reporting bugs

Dockerfile

@@ -13,10 +13,16 @@ RUN groupadd -g ${GID} ${UNAME} && \
 
 WORKDIR /app
 
-COPY requirements.txt .
-RUN pip install --no-cache-dir -r requirements.txt
+COPY requirements.txt requirements-dev.txt ./
+RUN pip install --no-cache-dir -r requirements.txt -r requirements-dev.txt
 
 COPY motion_mag.py .
+COPY tests/ tests/
+
+RUN chown -R ${UID}:${GID} /app
+
+ARG VERSION
+LABEL version=${VERSION}
 
 USER ${UNAME}
 

README.md

@@ -35,6 +35,10 @@ Phase-based motion magnification amplifies subtle motions invisible to the naked
   - [CLI Tool](#cli-tool)
   - [Notebook](#notebook)
   - [Tips](#tips)
+- [Development](#development)
+  - [Running Tests](#running-tests)
+  - [Versioning](#versioning)
+  - [Project Structure](#project-structure)
 - [References](#references)
 
 ---
@@ -213,6 +217,66 @@ Open the notebook and run all cells. By default, it downloads a sample face vide
 
 ---
 
+## Development
+
+### Running Tests
+
+All tests run inside Docker — no local Python dependencies needed:
+
+```bash
+# Run lint + unit tests (builds image automatically if not found)
+./test.sh
+
+# Force rebuild before testing
+./test.sh --build
+```
+
+**Tests** (`tests/test_motion_mag.py`) cover:
+- Phase normalization (unit magnitude, zero safety)
+- Flat-top temporal filter (DC passthrough, smoothing, edge cases)
+- Temporal phase extraction (constant phase, output shape)
+- `magnify_motions` smoke tests (shape, dtype, finite values)
+- `load_video` buffer safety
+- All CLI input validation error paths
+
+**Dev workflow:**
+1. Make your changes
+2. Run `./test.sh`
+3. If all tests pass, commit and open a PR
+4. CI runs lint + smoke tests automatically
+
+### Versioning
+
+Version is tracked in a `VERSION` file at the project root. `motion_mag.py` has `__version__` baked into the source (updated at release time).
+
+**To cut a release:**
+1. Update `VERSION` with the new version number
+2. Update `__version__` in `motion_mag.py`
+3. Update `CHANGELOG.md` — move items from `[Unreleased]` to `[X.Y.Z] - YYYY-MM-DD`
+4. Commit: `Release vX.Y.Z`
+5. Tag: `git tag -a vX.Y.Z -m "Release vX.Y.Z"`
+6. Push: `git push && git push origin vX.Y.Z`
+7. Rebuild Docker image: `./docker-build.sh`
+
+### Project Structure
+
+```
+motion_mag.py           # CLI tool
+MotionMagDtcwt.ipynb    # Jupyter notebook
+Dockerfile              # Docker image
+docker-build.sh         # Build + tag image
+test.sh                 # Run lint + unit tests (Docker)
+requirements.txt        # Runtime dependencies
+requirements-dev.txt    # Dev dependencies (pytest, ruff)
+tests/
+  test_motion_mag.py    # Unit tests
+docs/design/            # Architecture decision records
+VERSION                 # Single source of truth for version
+CHANGELOG.md            # Release history
+```
+
+---
+
 ## References
 
 1. Wadhwa, N., Rubinstein, M., Durand, F., & Freeman, W.T. (2013). [Phase-Based Video Motion Processing](https://people.csail.mit.edu/nwadhwa/phase-video/). *ACM Transactions on Graphics (SIGGRAPH)*, 32(4).

VERSION

@@ -0,0 +1 @@
+1.0.0

docker-build.sh

@@ -1,10 +1,14 @@
 #!/bin/bash
 set -e
 
+VERSION=$(cat VERSION)
+
 docker build \
     --build-arg UID="$(id -u)" \
     --build-arg GID="$(id -g)" \
     --build-arg UNAME="$(whoami)" \
-    -t motion-magnification-dtcwt .
+    --build-arg VERSION="${VERSION}" \
+    -t motion-mag-dtcwt:${VERSION} \
+    -t motion-mag-dtcwt:latest .
 
-echo "Built motion-magnification-dtcwt image as user: $(whoami) (uid=$(id -u), gid=$(id -g))"
+echo "Built motion-mag-dtcwt:${VERSION} (also tagged :latest)"

docs/design/dtcwt-hardening.md

@@ -0,0 +1,131 @@
+# Design Doc: DTCWT Motion Magnification — Project Hardening
+
+**Status: APPROVED**
+
+## Context
+
+The Motion Magnification Using 2D DTCWT project is a Python implementation of phase-based motion magnification (Anfinogentov & Nakariakov, 2016) using the Dual-Tree Complex Wavelet Transform. It's a single-file CLI tool (`motion_mag.py`) shipped via Docker.
+
+A code review identified the same gaps as the sibling EVM project: no unit tests, no versioning infrastructure, no design documentation, and minor bugs. This design doc covers retroactive architecture decisions and the hardening plan.
+
+## Goals and Non-Goals
+
+**Goals:**
+- Add unit tests for core functions
+- Fix bugs: `load_video` buffer overflow, `fps` truncation, remove dead `np.int` monkey-patch
+- Establish versioning infrastructure (VERSION file, Docker labels, changelog)
+- Modernize CI (Docker-based, add linting)
+- Add `test.sh`, `requirements-dev.txt`, and dev workflow documentation
+- Pin `dtcwt` upper bound
+- Document architecture decisions
+
+**Non-Goals:**
+- Refactoring into a pip-installable package
+- Integration tests with video output comparison
+- Regenerating demo GIFs (deferred to after GPU implementation)
+
+## Proposed Design
+
+### A. Architecture Decisions (retroactive)
+
+**Why 2D DTCWT over complex steerable pyramids?**
+The original Wadhwa et al. (SIGGRAPH 2013) phase-based method uses complex steerable pyramids, which require custom filter bank implementations. The 2D DTCWT (Selesnick et al., 2005) provides similar properties — near shift-invariance, directional selectivity (6 orientations per level) — with an off-the-shelf Python library (`dtcwt`). Anfinogentov & Nakariakov (2016) demonstrated this substitution works for motion magnification with comparable quality. The tradeoff: DTCWT has fixed 6 orientations per level vs steerable pyramids' configurable orientation count, but 6 is sufficient for general video motion.
+
+**Why flat-top window for temporal filtering?**
+The flat-top window (scipy.signal.windows.flattop) has a very flat passband, meaning signals within the passband are preserved with minimal amplitude distortion. For motion magnification, this matters because amplitude distortion in the temporal filter would non-uniformly scale phase changes, creating visible artifacts. The tradeoff: flat-top has wider transition band (slower rolloff) than alternatives like Hann or Hamming, so frequency selectivity is lower. For the typical use case (separating slow base motion from subtle detail motion), this is acceptable.
+
+**Why frame-to-frame complex division for phase extraction?**
+Phase could be extracted by computing `np.angle()` of each frame's coefficients directly, then differencing. Instead, the code computes `curr / prev` (complex division) and takes `np.angle()` of the ratio. This is more numerically stable because: (1) it avoids phase wrapping issues at ±π boundaries (the ratio's angle is always the correct small delta), (2) it works correctly even when individual coefficients have very small magnitude (the ratio's angle is still meaningful). The cumulative sum of these deltas gives the total phase evolution.
+
+**Why float64 throughout?**
+Verified: dtcwt produces `complex128` highpass coefficients (float64 real/imaginary pairs). Float64 roundtrip error is `4.4e-16` vs `3.6e-07` for float32. Phase extraction via cumulative complex division compounds errors across frames — with 300+ frames and 8 pyramid levels, float32 errors could become visible. Memory cost is 2x, but correctness is more important for a research tool.
+
+### B. Hardening Changes
+
+All follow the same patterns established in EVM hardening:
+
+**Bug fixes:**
+- Remove `np.int` monkey-patch — verified dtcwt 0.14.0 doesn't need it (numpy 1.26.4, no `np.int` references in dtcwt source)
+- Fix `fps` truncation: `int(cap.get(...))` → keep as float (verified `cv2.VideoWriter` accepts float fps, 3.2% speed error on 29.97fps videos)
+- Add `load_video` buffer guard: `if i >= frame_count: break`
+- `flattop_filter_1d`: guard against zero window size with `max(1, ...)`
+
+**Versioning:**
+- Add `VERSION` file (Approach B — build-time injection, same as EVM)
+- Stamp `__version__` in `motion_mag.py` at release time
+- `docker-build.sh` reads from `VERSION`, tags image, passes build arg for Docker label
+- Dockerfile gets `ARG VERSION` + `LABEL version=${VERSION}`
+
+**Testing:**
+- `requirements-dev.txt` with pytest and ruff
+- `tests/test_motion_mag.py` — unit tests:
+  - Tier 1 (strict): `normalize_phase` (unit magnitude property, zero-magnitude safety), `format_duration`
+  - Tier 2 (moderate): `flattop_filter_1d` (DC passthrough, smoothing effect), `extract_temporal_phases` (known constant phase → zero delta)
+  - Tier 3 (smoke): `magnify_motions` on tiny synthetic data (shapes, dtypes, finite values)
+  - Input validation error paths
+- `test.sh` — build once, run lint + tests in Docker
+- CI modernized: Docker-based, ruff lint, smoke tests only
+
+**CI modernization:**
+- Run inside Docker (matching dev workflow and EVM pattern)
+- Add ruff linting
+- Update `actions/checkout` to v6
+- Remove `actions/setup-python` (not needed when running in Docker)
+
+**Documentation:**
+- `CHANGELOG.md` (Keep a Changelog, starting fresh)
+- Design doc (this document)
+- `dtcwt` upper bound pinned (`<1`)
+- README updated with Development section (testing, versioning, project structure)
+- CONTRIBUTING.md updated with `test.sh` instructions
+
+### C. Future: GPU Acceleration (deferred, implement after hardening)
+
+**Approach**: Same hybrid pattern as the Visual-Mic project (`joeljose/Visual-Mic`):
+- CPU path: existing `dtcwt` library (unchanged)
+- GPU path: `pytorch_wavelets` (`DTCWTForward`) with PyTorch CUDA backend
+- `--gpu` flag switches between them, `--batch-size` controls GPU memory usage
+
+**Key considerations for motion magnification vs Visual-Mic:**
+- Visual-Mic does single-pass phase extraction (streaming) — motion magnification needs all pyramids in memory for temporal filtering
+- GPU batching applies to forward/inverse DTCWT transforms (the expensive part)
+- Temporal filtering (flat-top convolution on phase arrays) stays on CPU/numpy — it's already fast and operates on extracted phase angles, not full wavelet coefficients
+- `pytorch_wavelets` uses float32 internally — phase extraction precision may differ from CPU float64 path. Needs validation.
+
+**Dependencies**: `pytorch_wavelets` (installed from `git+https://github.com/fbcotter/pytorch_wavelets.git`), PyTorch with CUDA, `Dockerfile.gpu` based on PyTorch CUDA image.
+
+**After GPU implementation**: Regenerate demo GIFs using the GPU path for faster processing.
+
+## Alternatives Considered
+
+### Temporal filter: Flat-top vs ideal bandpass (FFT-based, as in EVM)
+
+| Approach | Pros | Cons | Verdict |
+|----------|------|------|---------|
+| Flat-top window convolution | Minimal amplitude distortion in passband, simple | Wider transition band, less frequency selectivity | **Chosen** — amplitude fidelity matters for phase-based method |
+| Ideal bandpass via FFT | Sharp frequency cutoff, matches EVM | Gibbs ringing, requires all frames for FFT | Rejected — ringing introduces phase artifacts |
+
+### Phase extraction: Complex division vs direct angle subtraction
+
+| Approach | Pros | Cons | Verdict |
+|----------|------|------|---------|
+| Complex division + cumsum | No phase wrapping, numerically stable | Slightly more computation | **Chosen** — correctness |
+| Direct `angle()` subtraction | Simpler code | Phase wrapping at ±π, needs unwrapping | Rejected — unwrapping adds complexity and failure modes |
+
+### GPU framework: pytorch_wavelets vs custom CuPy DTCWT
+
+| Approach | Pros | Cons | Verdict |
+|----------|------|------|---------|
+| pytorch_wavelets | Proven in Visual-Mic, maintained library, batched transforms | PyTorch is a large dependency, float32 only | **Chosen** (deferred) — minimal effort, proven approach |
+| Custom CuPy DTCWT | No PyTorch dependency, could use float64 | Massive implementation effort, no existing library | Rejected — not worth writing from scratch |
+
+## Tradeoffs and Risks
+
+- **No GPU acceleration (current)**: DTCWT is CPU-bound and slow for large videos. Accepted for now, GPU path planned as next phase.
+- **Memory intensive**: All frame pyramids stay in memory for temporal filtering. A 1080p 30s video at float64 needs ~42 GB for channels plus pyramid storage. Documented, not fixed.
+- **Removing `np.int` patch**: If someone uses an older dtcwt (<0.14.0), it will break. Mitigated by pinning `dtcwt>=0.12.0,<1`.
+- **GPU float32 vs CPU float64**: When GPU path is added, outputs may differ slightly from CPU. Will need validation and documentation.
+
+## Open Questions
+
+None for the hardening phase. GPU acceleration details to be resolved during that implementation.

motion_mag.py

@@ -23,10 +23,6 @@
 import numpy as np
 from scipy import ndimage, signal
 
-# NumPy compatibility: dtcwt uses np.int which was removed in NumPy 1.24+
-if not hasattr(np, 'int'):
-    np.int = np.int64
-
 import dtcwt
 
 
@@ -117,7 +113,7 @@ def flattop_filter_1d(data, width, axis=0, mode='reflect'):
     Returns:
         Filtered numpy array with same shape as input.
     """
-    window_size = round(width / 0.2327)
+    window_size = max(1, round(width / 0.2327))
     window = signal.windows.flattop(window_size)
     window = window / np.sum(window)
     return ndimage.convolve1d(data, window, axis=axis, mode=mode)
@@ -143,13 +139,15 @@ def load_video(path):
         frame_count = int(cap.get(cv2.CAP_PROP_FRAME_COUNT))
         width = int(cap.get(cv2.CAP_PROP_FRAME_WIDTH))
         height = int(cap.get(cv2.CAP_PROP_FRAME_HEIGHT))
-        fps = int(cap.get(cv2.CAP_PROP_FPS))
+        fps = cap.get(cv2.CAP_PROP_FPS)
 
         # Read all frames into a single array, then split channels
         frames = np.zeros((frame_count, height, width, 3), dtype=np.uint8)
         i = 0
         t_start = time.time()
         while cap.isOpened():
+            if i >= frame_count:
+                break
             ret, frame = cap.read()
             if not ret:
                 break
@@ -362,7 +360,7 @@ def main():
     print(f"  {frame_count} frames, {frame_size[0]}x{frame_size[1]}, {fps} fps")
 
     # --- Parameters ---
-    print(f"\nParameters:")
+    print("\nParameters:")
     print(f"  Magnification:   {args.magnification}x")
     print(f"  Filter width:    {args.width}")
     print(f"  DTCWT levels:    {args.nlevels}\n")