refactor: Tier 3 common/common.h coupling reduction + MSVC portability fix#5
Merged
refactor: Tier 3 common/common.h coupling reduction + MSVC portability fix#5
Conversation
Extract linalg type aliases into linalg/linalg_types.h and XML serialization templates into common/serialization.h. Lean common/common.h retains only the STL includes, Observation/StateIndex typedefs, and the math/constants.h include. Update all consumers (hmm.h, calculator.h, trainer.h, segmental_kmeans_trainer.h, numerical_stability.h, IO headers, src/common/common.cpp, tests/test_common.cpp) to include the appropriate new headers directly. Remove redundant common/common.h includes from test_pareto_distribution.cpp and test_rayleigh_distribution.cpp; both already get Observation transitively through their distribution header. Resolve all clean-build warnings: - benchmarks/src/simple_benchmark.cpp: cast nodiscard getProbability to void - benchmarks/src/libhmm_vs_hmmlib_benchmark.cpp: discard unused HMMLib viterbi return value - benchmarks/src/classic_problems_benchmark.cpp: remove unused local and private field ClassicHMMProblems problems - benchmarks/src/threeway_benchmark.cpp: discard unused HMMLib viterbi return value - tools/fb_contour_sweep.cpp, tools/hotspot_breakdown.cpp: mark FB_MAX_REDUCE_FORCE_PAIRWISE_MAX_STATES [[maybe_unused]] (only active under experimental preprocessor flags) Co-Authored-By: Oz <oz-agent@warp.dev>
std::setprecision, std::fixed, and std::put_time are used throughout the library (all 15 distribution toString() methods, file_io_manager.cpp, numerical_stability.cpp). Apple Clang includes <iomanip> transitively through <iostream> so the M1 build did not surface the missing include. MSVC and libstdc++ do not, causing C2039/C3861 build failures on Windows. Added with an explanatory comment to prevent future regressions during header-cleanup passes. Co-Authored-By: Oz <oz-agent@warp.dev>
OldCrow
added a commit
that referenced
this pull request
May 5, 2026
CMakeLists.txt: VERSION 3.3.0 -> 3.4.0 include/libhmm/libhmm.h: @Version 3.4.0 CHANGELOG.md: add [3.4.0] entry covering all three refactor phases: - Phase 1 (PR #4): code quality - weighted stats helper, duplicate preamble elimination, BaumWelch CC 32->13, common.h split - Phase 2 (PR #5): common.h fan-in 24 reduction - Phase 3 (PR #6): JSON serializer, IO alignment bug fix, operator>> decomposition, sanitization, sample files README.md: - Version badge: 3.1.2 -> 3.4.0 - Tests badge: 36/36 -> 38/38 - Add I/O section (JSON save_json/load_json, legacy XML) - Add samples/ to project structure - Add io/ to include/libhmm/ directory listing - Update tools/ and tests/ descriptions Co-Authored-By: Oz <oz-agent@warp.dev>
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.
Summary
Tier 3 of the code quality refactoring roadmap: splits the
common/common.hgod header to break the transitive linalg coupling that caused all 15 distribution headers to pull in ~695 SLOC of matrix/vector type definitions they never used.Includes a one-line MSVC portability fix discovered during Windows verification:
<iomanip>was being silently provided by Apple Clang through<iostream>but is required explicitly by MSVC and libstdc++.Changes
common/common.hsplit (Tier 3)include/libhmm/linalg/linalg_types.h(new): type aliasesMatrix,Vector,ObservationSet,StateSequence,ObservationListsandclear_*helpers — include only where linalg types are genuinely needed (calculators, trainers, HMM core, IO)include/libhmm/common/serialization.h(new):MatrixSerializer<T>andVectorSerializer<T>extracted fromcommon.h— include only fromsrc/hmm.cppand the IO layerinclude/libhmm/common/common.h: linalg and serialization content removed; now a lean header of core STL facilities andObservation/StateIndextype aliasescommon/common.hfor linalg types they don't useMSVC portability fix
#include <iomanip>tocommon.hwith an explanatory comment:std::setprecision/std::fixed/std::put_timeare used throughout the library but Apple Clang silently provides<iomanip>transitively through<iostream>while MSVC and libstdc++ do notVerification
<memory>template instantiationRoadmap
Tier 3 complete. Remaining:
hmm.cpp::operator>>decomposition (CC 18, length 258)Plan: https://app.warp.dev/drive/notebook/niAbtAHXXGBa2zwkv2pe6R
Session: https://app.warp.dev/conversation/ca60ff01-fa5e-45cf-be1a-5dd234ddd87c
Co-Authored-By: Oz oz-agent@warp.dev