Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve a Runtime Issue on Windows with MSVC #142

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

H-YWu
Copy link
Contributor

@H-YWu H-YWu commented Aug 9, 2024

Bug

On Windows the library compiled with MSVC will suddenly exit without any warning message when calling reach_for_the_arcs(), and results cannot be generated. It turns out that the bug is caused by the reset() function of the struct Profiler in the PoissonRecon code, which uses mutex. However, we need explicit configuration in MSVC if we need a multi-threading runtime library.

Changes

In CMakeListst.txt:

  • Add static multi-threading runtime library configuration for MSVC to ensure the PoissonRecon code runs successfully.
  • Set CMP0091 policy to NEW to support CMAKE_MSVC_RUNTIME_LIBRARY.

Testing

The changes were tested in the following environment:

  • Operating System: Windows 11
  • Compiler: MSVC 19.41.34117.0
  • IDE: Visual Studio 17 2022
  • Build Tool: CMake 3.27.7
  • Package Manager: Conda 24.3.0

Based on the same changes to CMakeListst.txt in https://github.com/H-YWu/reach-for-the-arcs-code/tree/windows-build:

All Python scripts in the scripts directory, except fig_many-shapes-experiment.py, were tested once (without ndc). There were several Runtime Warnings such as "MatrixRankWarning: Matrix is exactly singular" during the execution of some scripts, but results were generated successfully.

For https://github.com/H-YWu/gpytoolbox/tree/windows-build, I only tested fig_adding-spheres.py with replacing calling from rfta package with my modified build of gpytoolbox package. The results were generated successfully.

@sgsellan
Copy link
Owner

sgsellan commented Aug 9, 2024

Thank you so much for this contribution, Haoyang Wu! None of the usual maintainers of this library have regular access to Windows, so we really appreciate someone double-checking that things work over there as well. Let me run the automated checks to make sure no builds are broken, and I can merge afterwards

@H-YWu
Copy link
Contributor Author

H-YWu commented Aug 13, 2024

Thank you so much for this contribution, Haoyang Wu! None of the usual maintainers of this library have regular access to Windows, so we really appreciate someone double-checking that things work over there as well. Let me run the automated checks to make sure no builds are broken, and I can merge afterwards

Thanks for your quick reply, Dr. Sellán! You're welcome! I'm glad I could help ensure compatibility on Windows.

@sgsellan sgsellan added the merge after checks Once all unit tests and builds complete successfully, this PR will be merged into the main branch label Oct 29, 2024
@sgsellan sgsellan merged commit db3bcb0 into sgsellan:main Oct 29, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge after checks Once all unit tests and builds complete successfully, this PR will be merged into the main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants