changes to make it possible to build under Windows#7212
Draft
GitPaean wants to merge 14 commits into
Draft
Conversation
GitPaean
added
the
manual:irrelevant
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a set of portability fixes intended to make opm-simulators build successfully under Windows/MSVC (coordinated with related changes in opm-common and opm-grid).
Changes:
- Add/adjust includes and path-string conversions to avoid MSVC/Windows filesystem and standard library incompatibilities.
- Rework several OpenMP loops to MSVC-compatible “canonical” forms and address MSVC-specific template/type issues.
- Introduce Windows-specific fallbacks/workarounds for missing POSIX APIs/macros (signals,
ffs,S_ISDIR, allocation).
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_parallelwellinfo.cpp | Adds missing <numeric> include. |
| tests/test_outputdir.cpp | Converts std::filesystem::path to .string() for std::string variables. |
| tests/test_networkpressure.cpp | Adds MPI global fixture initialization for tests under MPI. |
| tests/models/test_tasklets_failure.cpp | Adds Windows process-spawn alternative to fork()-based failure test. |
| opm/simulators/wells/WellInterfaceFluidSystem.cpp | Adds <numeric> include for standard algorithms. |
| opm/simulators/wells/GasLiftStage2.cpp | Adds <numeric> include for standard algorithms. |
| opm/simulators/utils/satfunc/SatfuncConsistencyChecks.hpp | Refactors nested type / container choices to work around MSVC completeness issues. |
| opm/simulators/utils/satfunc/SatfuncConsistencyChecks.cpp | Updates implementation to match the moved violation-sample type. |
| opm/simulators/utils/ParallelFileMerger.cpp | Uses .string() instead of .native() for Windows narrow-string handling. |
| opm/simulators/linalg/getQuasiImpesWeights.hpp | Rewrites OpenMP range-for over chunks into index-based loops (MSVC/OpenMP compatibility). |
| opm/simulators/linalg/DILU.hpp | Adjusts OpenMP loop bounds to avoid MSVC /openmp:llvm ICE. |
| opm/simulators/flow/Transmissibility_impl.hpp | Rewrites chunk loop into index-based OpenMP-compatible form. |
| opm/simulators/flow/TracerModel.hpp | Rewrites chunk loops into index-based OpenMP-compatible forms. |
| opm/simulators/flow/SimulatorSerializer.cpp | Converts filesystem path to string when assigning to loadFile_. |
| opm/simulators/flow/rescoup/ReservoirCouplingSpawnSlaves.hpp | Adjusts argv construction API to accept narrow strings for Windows. |
| opm/simulators/flow/rescoup/ReservoirCouplingSpawnSlaves.cpp | Keeps narrow path string alive and avoids path::c_str() wchar_t issues on Windows. |
| opm/simulators/flow/RegionPhasePVAverage.cpp | Adds <numeric> include. |
| opm/simulators/flow/NonlinearSystemBlackOilReservoir.hpp | Works around Windows STRICT macro collision with enum values. |
| opm/simulators/flow/NlddReporting.hpp | Converts output paths to .string() for writers expecting std::string. |
| opm/simulators/flow/KeywordValidation.hpp | Adds <stdexcept> include for exception types. |
| opm/simulators/flow/GenericOutputBlackoilModule.cpp | Moves a local enum to namespace scope to avoid MSVC template instantiation issues. |
| opm/simulators/flow/FIBlackoilModel.hpp | Rewrites chunk loops into index-based OpenMP-compatible forms. |
| opm/simulators/flow/FacePropertiesTPSA_impl.hpp | Rewrites chunk loops into index-based OpenMP-compatible form. |
| opm/simulators/flow/ConvergenceOutputConfiguration.cpp | Adjusts regex tokenization to avoid MSVC initializer-list and iterator issues. |
| opm/simulators/flow/Banners.cpp | Introduces Windows memory/user retrieval (but still needs additional Windows guards to compile). |
| opm/models/utils/terminal.cpp | Guards signals not available on Windows/MSVC. |
| opm/models/utils/simulatorutils.cpp | Defines S_ISDIR for MSVC where missing. |
| opm/models/utils/parametersystem.hpp | Makes namespace stripping robust to MSVC className() prefixes. |
| opm/models/utils/alignedallocator.hh | Uses _aligned_malloc/_aligned_free on MSVC. |
| opm/models/pvs/pvsprimaryvariables.hh | Adds MSVC ffs() implementation via _BitScanForward. |
| opm/models/io/vtkmultiwriter.hh | Converts path::filename() to .string() for std::string. |
| opm/models/tpsa/tpsamodel.hpp | Rewrites chunk loop into index-based OpenMP-compatible form. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+36
to
+44
| @@ -37,13 +37,29 @@ | |||
| #include <thread> | |||
| #include <unistd.h> | |||
|
|
|||
| #if defined(_WIN32) | |||
| #define WIN32_LEAN_AND_MEAN | |||
| #define NOMINMAX | |||
| #include <windows.h> | |||
| #endif | |||
| @@ -58,7 +74,11 @@ void printPRTHeader(const int nprocs, const int nthreads, | |||
| const double megabyte = 1024 * 1024; | |||
| unsigned num_cpu = std::thread::hardware_concurrency(); | |||
| struct utsname arch; | |||
Comment on lines
134
to
138
| #if defined(_WIN32) | ||
| # undef STRICT | ||
| # undef RELAXED | ||
| #endif | ||
| enum class DebugFlags { |
Comment on lines
+142
to
+150
| if (argc > 1 && std::strcmp(argv[1], "--child") == 0) { | ||
| execute(); | ||
| return 0; // execute() is expected to exit(EXIT_FAILURE) before reaching here | ||
| } | ||
| std::cout << "Checking failure of child process with parent process" << std::endl; | ||
| const intptr_t status = _spawnl(_P_WAIT, argv[0], argv[0], "--child", | ||
| static_cast<const char*>(nullptr)); | ||
| assert(status == EXIT_FAILURE); // Check that the child exited with EXIT_FAILURE | ||
| } |
Comment on lines
80
to
84
| const char* user = getlogin(); | ||
| #endif | ||
| std::time_t now = std::time(0); | ||
| struct std::tm tstruct; | ||
| char tmstr[80]; |
- SatfuncConsistencyChecks: the std::array->std::vector change (an MSVC workaround) silently dropped the "violations_ always has NumLevels entries" invariant for moved-from objects, since moving a vector leaves it empty and a reused source would then index violations_[index(level)] out of bounds. Restore the source to NumLevels in the move ctor and move-assignment. - NonlinearSystemBlackOilReservoir.hpp: scope the Windows STRICT/RELAXED #undef with #pragma push_macro/pop_macro so it no longer strips the Windows SDK STRICT definition from the rest of the translation unit.
following bska's suggestion.
…NG guard to WIN32 and reuse shared MpiFixture
…ack so it builds on Windows
so the DebugFlags enumerators compile in _impl.hpp
…klet test, complete MSVC fixes in simulatorutils/terminal, restore std::array ViolationCollection
… PRT banner, fix include order
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.
together with OPM/opm-common#5236 and OPM/opm-grid#1048