M_PI is not defined by MSVC — three LTX-2.5 TUs fail the Windows build, revealed once #664's POSIX defect was fixed

[localai-bot]

Found by the operator gate on #677. That PR fixed the POSIX stat defect (#664) and the Windows job got further — then hit this. Two independent Windows defects were stacked, both from the LTX-2.5 lane, and the second was invisible until the first was removed.

The failure

windows-msvc-cpu on PR #677, step "Build and execute the native Windows CPU focused gate", after 17m52s:

src\vllm\model_executor\models\ltx2.cpp(72,58): error C2065: 'M_PI': undeclared identifier
src\vllm\model_executor\models\ltx2.cpp(72,15): error C2737: 'kBeta': const object must be initialized
src\vllm\model_executor\models\ltx2.cpp(557,69): error C2065: 'M_PI': undeclared identifier
src\vllm\model_executor\models\ltx2.cpp(572,76): error C2065: 'M_PI': undeclared identifier

M_PI is a POSIX extension, not standard C++. MSVC's <cmath> only defines it when _USE_MATH_DEFINES is defined before the include.

Scope — three files, all in this lane, and ltx2.cpp only hides the rest

git grep -l M_PI origin/main -- src include returns exactly three, all LTX-2.5:

• src/vllm/model_executor/models/ltx2.cpp — :72 std::sqrt(2.0 / M_PI) (the gelu-tanh constant), :557, :572 (M_PI / 2.0, the schedule)
• src/vllm/model_executor/models/ltx2_audio_vae.cpp
• src/vllm/model_executor/models/ltx2_video_vae.cpp

The job reports only ltx2.cpp because the build stops there. Fixing that file alone will move the error to the next TU. Re-derive the list at HEAD before acting — anchors in this tree go stale within a single PR — and use a positive control in the grep rather than trusting a null result.

Why the checker did not catch it

scripts/check-windows-portability.py is a source-pattern gate. It caught the unguarded <sys/stat.h> / ::stat shapes because those are greppable. M_PI is a compile error on one toolchain, not a pattern, so nothing static could have found it — only a native MSVC build. That is worth recording: the checker's green is not a claim that the tree builds on Windows.

It also compounds #584 — windows-msvc-* are skipped on main pushes, so main has no baseline and both defects landed unseen.

Preferred fix

Not a #define _USE_MATH_DEFINES. That is order-dependent (it must precede every <cmath>, including transitive ones) and it re-declares a POSIX extension rather than removing the dependency. Prefer a single portable constant — std::numbers::pi is available at the project's C++20 level (CMakeLists.txt:35 sets CMAKE_CXX_STANDARD 20), or one named constexpr in a shared header if a std::numbers include is unwelcome. One definition, three call sites, no macro ordering.

Whatever is chosen: the change must be value-identical. M_PI and std::numbers::pi_v<double> are both the nearest double to pi, so goldens should not move — assert that rather than assume it, and say so in the PR.

Gate

A green check-windows-portability.py is not sufficient evidence for this row; only the native windows-msvc-cpu / windows-msvc-vulkan jobs are. Note the runner pool is currently saturated (34 runs queued repo-wide), and a cancelled run prints as fail in gh pr checks without being one — read the job conclusion from the API.

Related: #664, #677 (must land first — this defect is only reachable once it does), #584, #680.

[localai-bot]

Correction to this issue's own scope — I was wrong in both directions

The implementer re-derived the TU list at HEAD with a positive control in the same command (git grep -n -E 'M_PI|std::numbers', which returned 8 std::numbers files, proving the grep was live). The result contradicts what I wrote above:

TU	M_PI uses	own #define?	actually breaks MSVC?
src/vllm/model_executor/models/ltx2.cpp	3	no	yes
tests/vllm/models/test_vocoder1d.cpp	2	no	yes — named nowhere above
src/vllm/model_executor/models/ltx2_audio_vae.cpp	4	yes	no
src/vllm/model_executor/models/ltx2_video_vae.cpp	0	yes (dead define)	no

My grep said -- src include. So it never looked in tests/, and missed a file that genuinely breaks the build — while counting two that compile fine behind their own #ifndef M_PI / #define M_PI 3.14159265358979323846. Fixing only what this issue originally named would have left windows-msvc-* red and burned another 20-minute cycle to find out.

That is the project's own standing trap — never assert absence from a failed grep — reproduced by the issue whose whole subject is a missed portability defect. The scope of a search is part of the claim.

The gate that would have caught this already existed, and was already red

tests/scripts/test_check_windows_portability.py::test_real_tree_uses_portable_windows_allocation_and_math asserts no \bM_PI\b under src/ or tests/. It was RED at the base SHA, naming all four files, before anyone touched anything.

It never fired because that suite is executed by no workflow — #680 / #646. So LTX-2.5 landed M_PI on top of a gate that was already objecting, and nothing was listening. Combined with #584 (windows-msvc-* skipped on main pushes, so no baseline), that is two independent blind spots pointing the same way, and it is why two Windows defects stacked before either was noticed.

Fix

PR #732, using std::numbers::pi_v<double> at all four sites and removing the two hand-rolled defines (extra spellings of one constant — #687's disease). Value identity established three ways, including bit-pattern equality 400921fb54442d18 across all three spellings.

MSVC's header condition was reproduced on Linux with -U_GNU_SOURCE -D_ISOC99_SOURCE, which removes glibc's M_PI from <cmath>: it reported ltx2.cpp at 72:58, 557:69, 572:76 — the same line and column as the CI log — plus test_vocoder1d.cpp 106:21 and 121:21, and nothing for the two guarded VAEs, confirming their guards by measurement rather than by reading.

#732 and #677 must both land; neither turns the lane green alone.