Feat/fixed step ref#20
Open
johnaoga wants to merge 20 commits into
Open
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>
Add DIC_analysis_parallel_input.fixed_step_ref (default 0 = off, existing behavior unchanged). When > 0, matlab_compute_seed_segment caps every seed segment at N frames, so the segment loop performs a reference change (seed propagation + chain composition back to the global reference in the exact_matlab_* path) every N frames regardless of seed quality — MATLAB ncorr step analysis semantics (stepanalysis.step / step_ref_change). Verified on S09 trial 7 (150 frames, step 10): 15 segments per tracking call, segment boundaries at frames 11, 21, ... matching MATLAB's recorded dispinfo.stepanalysis (enabled=1, type='seed', auto=1, step=10). Co-Authored-By: Claude Fable 5 <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
1 participant
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.
No description provided.