feat: CppNCorr newversion — in-memory API, params, tests, docs, CI#12
Merged
Conversation
Set CMAKE_CXX_STANDARD 17 / CXX_STANDARD_REQUIRED / CXX_EXTENSIONS OFF project-wide at the top of CMakeLists.txt (previously only set per-target). Add CMakeOptions.md documenting the FORCE_FETCH_DEPENDENCIES option, the language requirement, compile flags, targets, all third-party dependencies and their fetch versions, and the known pre-existing Array2D.h warnings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The de-facto main DIC executable is proxyncorr (folder discovery + full getopt CLI + config file + JSON/binary/video output). Audited its flags and added the post-DIC output flags required by the spec: - --export-video forces displacement/strain video output (backing exists) - --export-strains guarantees strain fields are written (backing exists) - --change-perspective <mode>: eulerian is functional (current default); lagrangian/other modes print 'not yet implemented' and exit cleanly. Documented the new flags in --help under a POST-DIC OUTPUT FLAGS section. Also fixed a pre-existing -Wall build blocker: two calls to the overloaded DIC_analysis_sequential were ambiguous (DIC_analysis_parallel_input is implicitly convertible to DIC_analysis_input). Disambiguated via explicit function-pointer overload selection, marked with FIXME(newversion). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Harden discover_frames() in proxyncorr: - length-safe image-extension check via has_image_extension() helper (was using raw substr() that could misbehave on short names); unified supported list: .png .tif .tiff .bmp .jpg .jpeg. - guard against empty dirent names; skip nested sub-directories (S_ISDIR). - natural (numeric-aware) sort so unpadded frame names order correctly (frame_2 < frame_10), in addition to zero-padded names. - richer error message including strerror(errno) when the folder cannot be opened. - documented the expected naming convention / reserved roi.png & ref.png and supported formats in a block comment above the function. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add include/ncorr/session.h and src/session.cpp defining a dependency-light
in-memory DIC interface so callers (e.g. CPPxDIC's proxyncorr) can push image
data directly without writing frames to disk:
- ImageBuffer: thin non-owning view {const uint8_t* data, width, height,
channels} with valid()/size_bytes() helpers.
- NcorrSession: set_reference(), optional set_roi(), process_frame() ->
DICResult, has_reference(). PIMPL keeps heavy ncorr internals out of the
public header.
- DICResult: flat row-major u/v (+ optional corrcoef) as std::vector<double>,
so callers need no internal ncorr types.
- SessionConfig: tuneable params whose names match the upcoming Config.
Stub bodies validate inputs and behave gracefully (process_frame returns an
explicit 'not yet implemented' DICResult / prints a notice). Wired session.cpp
and session.h into the ncorr library target. TODO/FIXME(newversion) markers
point to proxyncorr.cpp's file-based pipeline as the model to follow.
Verified: compiles clean under -Wall -Wextra and links/runs from an external
caller TU.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e chain
Add a config-file system from scratch (no new dependency):
- include/ncorr/config.h: struct Config holds every tuneable ncorr parameter
with its compiled-in default; the field names are the canonical config keys.
- include/ncorr/ini.h: tiny header-only INI parser (key=value, [sections],
'#'/';' full-line and inline comments, quoted values, CRLF-safe, typed
getters with error messages). Written in-tree because no header-only parser
was vendored and to avoid adding TOML/INI libraries.
- src/config.cpp: load_config_file() overlays INI values onto a Config,
matching keys to fields one-to-one.
- config/default.cfg: documents the INI format choice and mirrors every Config
field with its default and a one-line comment; keys match Config exactly.
Integrate the three-tier chain into proxyncorr:
tier 3 compiled defaults (seeded from ncorr::Config)
-> tier 2 config file (legacy short keys + canonical INI overlay; falls back
to config/default.cfg when present)
-> tier 1 CLI args.
Malformed config values are reported and abort cleanly.
Verified: config overrides defaults, CLI overrides config, bad values error
out; library and proxyncorr build clean under -Wall -Wextra.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ntation Scan of the source tree and existing ad-hoc harnesses; proposes unit, integration and end-to-end tests (config/INI parser, NcorrSession, folder reader, Image2D, Array2D, interpolation, RGDIC, strain, ROI, units/perspective; load->DIC->strain->export pipeline; ohtcfrp e2e smoke + golden value). Lists open questions requiring confirmation before section 5b: test framework choice (recommend Catch2 v3 via the existing FetchContent pattern, otherwise in-tree asserts + CTest), extracting the folder-reader helpers for testability, and the e2e golden control point/tolerance. No test code written yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ader Move discover_frames, has_image_extension and natural_less out of test/src/proxyncorr.cpp into a new header-only unit include/ncorr/frame_reader.h (namespace ncorr, inline functions) so they can be unit-tested in isolation without compiling the driver. Behaviour is unchanged; proxyncorr.cpp now includes the header and uses the functions via its existing 'using namespace ncorr'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire up the test infrastructure and the dependency-light unit tests.
Framework: Catch2 v3 via FetchContent (matches the repo's existing
FetchContent dependency pattern). ALL test infra (Catch2 fetch,
enable_testing, test targets, CTest registration) is gated behind
PROJECT_IS_TOP_LEVEL AND BUILD_TESTING in the root CMakeLists, with a
manual PROJECT_IS_TOP_LEVEL fallback for CMake < 3.21, so it never leaks
into the parent CPPxDIC add_subdirectory build. BUILD_TESTING defaults ON
only when CppNCorr is top-level.
Unit targets (ncorr_unit_tests) compile only the small dependency-free
sources (ini.h, config.cpp, session.cpp, frame_reader.h) so they build
without OpenCV/FFTW/SuiteSparse/BLAS:
- test_ini.cpp: parsing, comments, quotes, sections, CRLF, typed
getters (valid + invalid), missing file.
- test_config.cpp: compiled defaults, file overlay, missing-file/no-op,
invalid value throws, and key/field parity against config/default.cfg.
- test_frame_reader.cpp: has_image_extension, natural_less, discover_
frames sorting/exclusions/empty/missing-folder.
- test_session.cpp: ImageBuffer validity, reference-required precondition,
invalid-reference rejection, geometry mismatch, and the current stub
contract (must be updated when in-memory DIC lands).
Tests registered with CTest via catch_discover_tests. Catch2 build
artifacts are redirected to the build tree (and gitignored) so the source
lib/ stays clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ncorr_engine_tests integration cases that link the full ncorr engine
and exercise the pipeline stages on the lightweight ohtcfrp fixture
(reference + one deformed frame, scalefactor 1, radius 30, single thread
to stay CI-friendly):
- pipeline_load_to_dic: load -> DIC_analysis; assert one displacement
field with positive dimensions and finite in-ROI displacements.
- pipeline_perspective_units: change_perspective + set_units preserve
field dimensions and record units/units_per_pixel.
- pipeline_dic_to_strain: strain_analysis yields a finite in-ROI strain
field of the expected shape.
- pipeline_config_drives_run: tuneable params land on DIC_analysis_input.
Assertions are structural (dimensions, finiteness) with exact-value
regression deferred to the e2e test. The fixture image directory is
injected via the NCORR_FIXTURE_DIR compile definition.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ncorr_engine_tests e2e cases running the full file-based pipeline
(load -> DIC_analysis -> change_perspective -> set_units -> strain_analysis)
on the ohtcfrp fixture with a reduced frame count (reference + 2 frames)
to stay CI-friendly:
- e2e_ohtcfrp_smoke: the run completes and produces one displacement and
one strain field per deformed frame; the in-ROI mean displacement
magnitude is finite.
- e2e_ohtcfrp_known_value: asserts the mean in-ROI |displacement| on the
final frame matches a recorded GOLDEN baseline within a tolerance band.
Captured baseline (macOS/AppleClang, OpenCV 4.13, single-threaded;
identical across 3 consecutive runs):
mean in-ROI |displacement| (last frame) = 0.180423 mm
tolerance = +/-0.02 mm (~11%), generous on purpose so the guard is
robust across platforms/compilers/thread counts while still catching a
real behavioural regression.
Set NCORR_E2E_CAPTURE=1 to re-capture the value instead of asserting.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
No HPC or Docker assets existed in the repo previously, so introduce a
deploy/ directory with the deployment recipes for the main DIC tool
(proxyncorr):
- deploy/Dockerfile: single-stage Ubuntu 24.04 image that installs all
dependencies (OpenCV, FFTW, SuiteSparse, OpenBLAS/LAPACK,
nlohmann/json, OpenMP) from the distro and builds the proxyncorr
executable (BUILD_TESTING=OFF so the Catch2 suite is excluded).
ENTRYPOINT is proxyncorr. Build context is the repo root
(docker build -f deploy/Dockerfile .).
- .dockerignore (repo root, where Docker reads it): keeps local build
trees and caches out of the build context.
- deploy/run_slurm.sh: sbatch script for a single-node OpenMP DIC run,
binding OMP_NUM_THREADS to the allocated CPUs (OMP_PROC_BIND=spread,
OMP_PLACES=cores).
- deploy/README.md: explains each asset, its target environment, Docker
and SLURM usage, and how to run the image under Apptainer/Singularity.
The Dockerfile's native build sequence (library target + proxyncorr via
the test tree) was verified to build the executable cleanly; the Docker
daemon was unavailable in this environment to run the container build
itself.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add .github/workflows/ci.yml triggered on push and PR to main and
newversion, with three jobs on ubuntu-latest (GCC), all using apt-provided
dependencies (no GPU, no source builds, no large downloads):
- build: cmake configure + build of the library and test targets.
- test: build the test targets, run the fast unit tests (ctest -L unit),
then the integration tests on the ohtcfrp fixture (ctest -L integration,
generous timeout). The very slow e2e cases are excluded from CI and run
locally/on demand.
- lint: clang-format --dry-run --Werror on the C/C++ files ADDED on this
branch (legacy files are not reformatted wholesale).
Supporting changes folded into this CI unit:
- .clang-format (Google-based, 100 col, 4-space indent) defining the style
the lint job enforces for new code.
- test/tests.cmake: label discovered tests (unit/integration/e2e) so CI can
select them with ctest -L.
- reformat the new test/header sources added on this branch to satisfy the
new lint rule (whitespace only; no behavioural change; unit tests still
pass).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the stub bodies with a real implementation that mirrors the
file-based pipeline in proxyncorr.cpp: each ImageBuffer is wrapped in an
owning cv::Mat and turned into an ncorr::Image2D; an optional mask becomes
a ROI2D (full-frame default); process_frame() builds a DIC_analysis_input
{ref, def} and runs DIC_analysis(), copying the resulting Disp2D u/v/cc
arrays into the flat DICResult vectors (NaN outside the ROI).
Results are the native Lagrangian displacements in pixels on the reduced
analysis grid (DICResult.width/height are the array dims, not the image
size); the correlation coefficient field is now populated. set_roi()
validates geometry against the reference, and a new reference clears any
prior ROI. The public session.h interface is unchanged, so CPPxDIC's
proxyncorr can call it without modification.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…engine target session.cpp now links the full DIC engine, so its tests can no longer live in the dependency-light ncorr_unit_tests target. Move test_session.cpp (and drop the standalone src/session.cpp compile) into ncorr_engine_tests, which links libncorr + OpenCV. The contract cases (validation, reference-required, geometry mismatch) are tagged [session] and discovered under the "unit" label; a new [integration] parity test loads the ohtcfrp fixture and asserts NcorrSession::process_frame reproduces the file/Image2D + DIC_analysis path within 1e-6. Replaces the obsolete stub-contract test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p overload Implements the three remaining proxyncorr TODOs: - --change-perspective: wire up "lagrangian" alongside "eulerian" (default). Eulerian keeps the existing change_perspective() conversion; lagrangian leaves the native DIC output untouched. Video filenames now reflect the active perspective. Unknown values error out cleanly. Default behaviour (no flag) is unchanged. - --export-strains: add a dedicated standalone CSV strain export (save_strains_csv): per strain frame, three row-major grid CSVs (exx/eyy/exy), values inside the ROI at 9 sig-digits and "nan" outside, written to <output>/strains_csv. The JSON/binary bundle behaviour is kept. - DIC_analysis_sequential: replace the function-pointer overload-selection workaround with the unambiguous 3-arg overload DIC_analysis_sequential(input, seeds, optimized) for both the seeded and auto-seed sequential paths. Verified by a CLI run on the ohtcfrp fixture (lagrangian + strain CSVs produced, exit 0) and the existing test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The lint job installed clang-format via apt, whose major version differs from the 18.1.8 used to format the branch's added files, so it reported spurious violations (e.g. include/ncorr/ini.h) on code that is actually clean. Install the pinned version via pip and invoke that binary explicitly. Scope is unchanged (files added on the branch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ini.h, session.h, session.cpp and test_session.cpp were originally formatted with a newer clang-format (v21); the CI lint job pins 18.1.8 and flagged them. Reformat with 18.1.8 so the lint job (which checks branch-added files) passes. Whitespace/wrapping only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
This branch hardens and extends CppNCorr (the standalone C++ 2D DIC engine that
is also vendored as a submodule of CPPxDIC) without changing the behaviour of the
existing image-folder DIC pipeline. Work spans repository hygiene, CLI auditing,
input handling, a parameter override chain, a real test suite, documentation,
deployment assets, and CI.
Section 1 — Repository hygiene
flag, dependency, and target in
CMakeOptions.md.chore: enforce C++17, document CMake options.Section 2 — Outputs / CLI (the DIC executable is
proxyncorr)proxyncorrCLI and added post-DIC output flags:--export-video,--export-strains,--change-perspective <mode>. Flags whosebacking code exists are wired to existing behaviour; modes without dedicated
code print a clear "not yet implemented" notice and exit cleanly. (
ncorristhe static library target;
proxyncorris the actual DIC tool.)feat(ncorr): audit CLI; stub missing post-DIC output flags.Section 3 — Inputs
supported formats, reserved
roi.png/ref.png, hidden files, sub-directories,empty/missing folders) and documented the naming convention.
fix(input/images): harden image-folder reader edge cases.discover_frames,has_image_extension,natural_lessinto the reusable header-onlyinclude/ncorr/frame_reader.h;proxyncorr.cppnow includes it. Nobehavioural change.
refactor(input/images): extract frame-reader helpers into reusable header.NcorrSessionAPI(
include/ncorr/session.h+src/session.cpp) withImageBuffer,DICResult,SessionConfig, and a PIMPL session class. Interface is final; bodies arestubs (validate inputs, return a graceful "not yet implemented" result).
feat(input/memory): add in-memory NcorrSession API (stub).Section 4 — Parameters
defaults:
include/ncorr/config.h(struct Config+load_config_file),header-only INI parser
include/ncorr/ini.h, andconfig/default.cfgwhosekeys match
Configfield names exactly.feat(params): implement CLI > config-file > compiled-defaults override chain.Section 5 — Tests
TESTS_PLAN.md(committed earlier, plan approved).docs(tests): add TESTS_PLAN.md — awaiting confirmation before implementation.(approved new dependency, matches the repo's existing FetchContent pattern).
test/unit/, targetncorr_unit_tests, dependency-light):INI parser, Config + key/field parity, frame-reader helpers, and the
NcorrSessionstub contract — 24 test cases / 130 assertions.test/integration/, targetncorr_engine_tests):load→DIC, change_perspective+set_units, DIC→strain, and config-driven input
on the
ohtcfrpfixture — 4 test cases.test/e2e/): full pipeline onohtcfrp(reference + 2frames) with a captured golden baseline — 2 test cases.
catch_discover_tests, labelledunit/integration/
e2e). All 30 CTest tests pass locally.test(unit): ...,test(integration): ...,test(e2e): ....Section 6 — Documentation
docs/quickstart.md,docs/developer_guide.md,docs/user_guide.md.docs: add quickstart guide,docs: add developer guide,docs: add user guide.Section 7 — Parallel / cluster / Docker
deploy/directory (no such assets existed before):Dockerfile(builds
proxyncorron Ubuntu 24.04 with apt deps),run_slurm.sh(single-node OpenMP sbatch script),
deploy/README.md, and a repo-root.dockerignore. The Dockerfile's native build sequence was verified to buildproxyncorrcleanly (the Docker daemon was unavailable to run the containerbuild itself in the prep environment).
chore(deploy): organise parallel/cluster/Docker scripts under deploy/.Section 8 — CI/CD
.github/workflows/ci.yml(push + PR tomain/newversion):build,test(unit + integration onohtcfrpvia ctest), andlint(
clang-format --dry-runon added files). Added a.clang-format. Fast: GCCon ubuntu-latest, apt deps, no GPU/large downloads; slow e2e excluded from CI.
ci: add GitHub Actions workflow for build, test, lint.Stubs and TODO markers
src/session.cpp):process_framereturns"NcorrSession::process_frame not yet implemented".src/session.cpp:10TODO(newversion):implement bodies following thefile-based pipeline.
src/session.cpp:58TODO(newversion):convert reference toImage2D.src/session.cpp:70TODO(newversion):convert mask toROI2D.src/session.cpp:94FIXME(newversion):in-memory DIC not implemented.session_stub_contractunit test and add thedeferred
pipeline_session_matches_folderintegration test fromTESTS_PLAN.md.test/src/proxyncorr.cpp):--change-perspectivenon-eulerianmodes print "not yet implemented"(
FIXME(newversion):near line 619).--export-strainsrelies on the JSON/binary bundle;TODO(newversion):(near line 608) to add a dedicated standalone strain export format.
FIXME(newversion):notes document theDIC_analysis_sequentialoverload-disambiguation already handled explicitly.
(
kGoldenMeanDispMag = 0.180423 mm, tolerance±0.02 mm). Re-capture withNCORR_E2E_CAPTURE=1.Items that were confirmed / need awareness
frame_reader.h— confirmed.|displacement| on the final frame). Reviewers may wish to confirm the chosen
metric and ±0.02 mm band.
Critical: test infra is guarded; parent build is unaffected
All test infrastructure (Catch2 FetchContent,
enable_testing(), the testtargets, and CTest registration) is gated behind
if(PROJECT_IS_TOP_LEVEL AND BUILD_TESTING)in the rootCMakeLists.txt(with a manual
PROJECT_IS_TOP_LEVELfallback for CMake < 3.21, andBUILD_TESTINGdefaulting ON only when top-level). Verified by configuring afake parent project that does
add_subdirectory(...): CppNCorr printedtests skipped (PROJECT_IS_TOP_LEVEL=OFF, BUILD_TESTING=OFF), no Catch2 wasfetched, no test targets were generated, and the
ncorrlibrary still built.How to review
TESTS_PLAN.md, thendocs/quickstart.md,docs/developer_guide.md,docs/user_guide.md.that
add_subdirectory()s this repo and confirm the "tests skipped" messageand that no Catch2/test targets appear.
include/ncorr/config.h,include/ncorr/ini.h,config/default.cfg, and the overlay wiring intest/src/proxyncorr.cpp.include/ncorr/session.h(final interface) andsrc/session.cpp(stub bodies + TODO markers).deploy/Dockerfile,deploy/run_slurm.sh,deploy/README.md,.github/workflows/ci.yml,.clang-format.Commit list (
git log --oneline main..newversion)🤖 Generated with Claude Code