Fixes build and tests on Windows #1519

kennyweiss · 2025-03-12T02:51:27Z

Summary

This PR is a bugfix for the build on Windows
It fixes:
- the MIR build, which required some exports for static members
- MIR tests that depend on the compile time path to the data directory
- A bug with the quicksort algorithm for some data orderings and adds a unit test
- Some quest tests when mfem does not have MPI
- Some quest lambdas that take a constexpr -- MSVC appears to require static constexpr w/ c++14. This might be relaxed in c++17
I also cleaned up/updated some sidre::Group implementations to use the iterator interface while working on sidre test bugfixes
Update: Passes all builds and tests in our Windows TPL CI -- https://github.com/LLNL/axom/actions/runs/13846008225

Status:

The code + tests + examples currently build w/ our msvc configuration
The following tests are still currently failing (checked when fixed):
- 85 - sina_Document (Failed)
- 86 - sina_ID (Failed)
- 87 - sina_Record (Failed)
- 88 - sina_Relationship (Failed)
- 89 - sina_Run (Failed)
- 170 - sidre_read_write_userdefined_data (Failed)
- 171 - sidre_mfem_datacollection (Failed)
- Update: All fixed!
The sina failures appear to be related to data that is moved. I will try to fix w/ ubsan
- Update: Fixed. There was also an error related to trying to rename a tmp file. Windows does not allow this if the destination already exists.
The sidre failures appear to be related to MFEMSidreDataCollection::LoadExternalData(). Perhaps related to changes from:
- Fix MFEMSidreDataCollection::LoadExternalData and add test cases for various LoadExternalData functions #1473
- Allow IOManager to load external data at specific locations in Sidre hierarchy. #1479
- Piecewise version of loadExternalData #1494
- Update: The sidre failures were due to loading external data into the wrong group when not going through spio

kennyweiss · 2025-03-13T02:03:10Z

/style

kennyweiss · 2025-03-13T17:41:01Z

/style

@white238 -- I am not sure that this worked. It seemed to trigger an action, but for some reason, the action was cancelled.
See: https://github.com/LLNL/axom/actions/runs/13825505635

BradWhitlock

Looks good to me.

BradWhitlock · 2025-03-13T18:37:07Z

src/axom/mir/clipping/ClipCases.h

@@ -88,87 +90,87 @@ namespace visit {
 #define NOCOLOR 122

 // Tables
-extern VISIT_VTK_LIGHT_API int numClipCasesHex;


Thanks for fixing the export macro. I don't know how I overlooked that.

(there were a lot of files :) )

kennyweiss · 2025-03-13T21:01:02Z

scripts/vcpkg_ports/umpire/v2024.07.0-chrono-include.patch

+ #ifndef UMPIRE_event_HPP
+ #define UMPIRE_event_HPP
+
+#include <chrono>


@davidbeckingsale -- My local machine did not need this on Windows, but our CI needed this for [email protected].
I saw that it was recently added to umpire@develop -- LLNL/Umpire#934

kennyweiss · 2025-03-13T21:56:47Z

src/axom/sina/core/ID.cpp

+  : value(extractIDFromObject(parentObject, localName_, globalName_))
+{
+  std::swap(localName, localName_);
+  std::swap(globalName, globalName_);
+}


@doutriaux1 @bgunnar5 @gwaegner -- this was tricky to track down.

Presumably msvc has a different order of processing the arguments than clang and other compilers, or it wiped out the strings after moving them, rather than leaving them unchanged.

The call to extractIDFromObject was happening after localName_ and globalName_ were already moved.

kennyweiss · 2025-03-13T22:45:55Z

src/axom/sidre/core/MFEMSidreDataCollection.cpp

+    if(!group_name.empty())
+    {
+      grp->loadExternalData(path);
+    }
+    else
+    {
+      m_bp_grp->loadExternalData(path);
+    }


@white238 and @nselliott -- This was necessary to get loadExternalData() working in our Windows configuration, which has MPI for axom and a serial mfem.

I am not sure if we have test coverage for the if part (where the users provide a group_name)

The former is not available in msvc compiler

Fixes some MIR tests that check for the existence of data paths.

Chooses a better partition for the quick sort so it doesn't run out of stack space.

Limit the scope of the saved and loaded variables and use EXPECT_EQ instead of EXPECT_TRUE.

There's no guaranteed on the order of operations that constructor parameters are accessed. This was using references to strings before/after they were moved of the IDType constructor.

…ists

... when MFEM is built without MPI support. Instead of loading from the hierarchy root when the user doesn't specify a group, load from the blueprint group.

This will be changed through uberenv in a separate PR.

kennyweiss added bug Something isn't working Build system Issues related to Axom's build system maintenance Issues related to code maintenance Windows Related to Windows development labels Mar 12, 2025

kennyweiss requested review from white238, cyrush, BradWhitlock, doutriaux1, rhornung67, nselliott, publixsubfan, gunney1, bgunnar5 and gwaegner March 12, 2025 02:51

kennyweiss self-assigned this Mar 12, 2025

kennyweiss added this to the April 2025 Release milestone Mar 12, 2025

bgunnar5 requested a review from HaluskaR March 13, 2025 16:14

BradWhitlock approved these changes Mar 13, 2025

View reviewed changes

kennyweiss commented Mar 13, 2025

View reviewed changes

bmhan12 approved these changes Mar 18, 2025

View reviewed changes

kennyweiss added 6 commits March 19, 2025 09:25

Replace posix drand48 with axom::utilities::random_real

3a9f4e8

The former is not available in msvc compiler

Exports some static const members in mir component

97a3660

Bugfix -- use native path for AXOM_DATA_DIR

b88044d

Fixes some MIR tests that check for the existence of data paths.

Bugfix and unit tests for qsort algorithm

f66b9a3

Chooses a better partition for the quick sort so it doesn't run out of stack space.

Some more fixes for mir tests on Windows

1585e24

Fixes some quest examples for MSVC

b125f47

kennyweiss and others added 13 commits March 19, 2025 09:25

Slight tweak to docs

6b04a36

Slight rework of sidre_read_write_userdefined_data test

c8fc2a0

Limit the scope of the saved and loaded variables and use EXPECT_EQ instead of EXPECT_TRUE.

Use iterator interface in several functions in sidre::Group

98936be

Bugfix: Adds missing return value to function in non-umpire configs

6d6d8a2

style

11c999e

Updates vcpkg SHA to 2025.02.14 release

3bbb3e0

Adds a patch to [email protected] to add a missing include

4d2bc82

Fixes an msvc warning related to __has_attribute

00bb588

Fixes another constexpr->static constexpr for lambdas in msvc

83a80c9

Bugfix: Moved strings were being used

e0d2077

There's no guaranteed on the order of operations that constructor parameters are accessed. This was using references to strings before/after they were moved of the IDType constructor.

Bugfix: Windows does not allow rename when the destination already ex…

f51c78f

…ists

Bugfix for MFEMSidreDataCollection::LoadExternalData()

fcb99dc

... when MFEM is built without MPI support. Instead of loading from the hierarchy root when the user doesn't specify a group, load from the blueprint group.

style

b116749

kennyweiss force-pushed the bugfix/kweiss/windows-build branch from 99d4ebf to b116749 Compare March 19, 2025 16:37

Update clang-format on tioga to get past CI

29a9f80

This will be changed through uberenv in a separate PR.

kennyweiss merged commit d4dc2d1 into develop Mar 20, 2025
13 checks passed

kennyweiss deleted the bugfix/kweiss/windows-build branch March 20, 2025 04:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes build and tests on Windows #1519

Fixes build and tests on Windows #1519

kennyweiss commented Mar 12, 2025 •

edited

Loading

kennyweiss commented Mar 13, 2025

kennyweiss commented Mar 13, 2025

BradWhitlock left a comment

BradWhitlock Mar 13, 2025

kennyweiss Mar 13, 2025

kennyweiss Mar 13, 2025

kennyweiss Mar 13, 2025

kennyweiss Mar 13, 2025

Fixes build and tests on Windows #1519

Fixes build and tests on Windows #1519

Conversation

kennyweiss commented Mar 12, 2025 • edited Loading

Summary

Status:

kennyweiss commented Mar 13, 2025

kennyweiss commented Mar 13, 2025

BradWhitlock left a comment

Choose a reason for hiding this comment

BradWhitlock Mar 13, 2025

Choose a reason for hiding this comment

kennyweiss Mar 13, 2025

Choose a reason for hiding this comment

kennyweiss Mar 13, 2025

Choose a reason for hiding this comment

kennyweiss Mar 13, 2025

Choose a reason for hiding this comment

kennyweiss Mar 13, 2025

Choose a reason for hiding this comment

kennyweiss commented Mar 12, 2025 •

edited

Loading