aaspip / pyekfmm

A python package for 3D fast-marching-based traveltime calculation and its applications in seismology
GNU General Public License v3.0
51 stars 11 forks source link

Fix: Add M_PI definition to eikonal.c and eikonalvti.c for MSVC compatibility on Windows #5

Closed Cleaf-y closed 7 months ago

Cleaf-y commented 7 months ago

This pull request addresses a compilation issue encountered when building the project on Windows 11 x64 23H2 using MSVC 143. The problem stemmed from the M_PI macro being undeclared in the eikonal.c file, which led to compilation failures.

Changes Made:

Rationale:

The MSVC compiler, unlike some other compilers, does not define M_PI by default. This lack of definition caused build failures on Windows. By adding a conditional definition, we ensure broader compatibility and a smoother build process on Windows systems, especially for those using MSVC.

Testing:

The modifications have been tested on a Windows 11 x64 23H2 system with MSVC 143 as the build tool. After applying these changes, the project builds successfully without any M_PI related errors.

This contribution aims to enhance the project's portability and ease of build on Windows platforms, potentially benefiting other Windows users who may face similar compilation issues.

chenyk1990 commented 7 months ago

More insights into compiling pyekfmm in Windows are very welcome.

Please feel free to reach out to me.

Also, some hints about connecting to Github user "Cleaf-y" are very helpful for me to gain more experience in Windows compilation.

-Yangkang

On Sat, Mar 9, 2024 at 11:49 PM Cleaf-y @.***> wrote:

This pull request addresses a compilation issue encountered when building the project on Windows 11 x64 23H2 using MSVC 143. The problem stemmed from the M_PI macro being undeclared in the eikonal.c file, which led to compilation failures. Changes Made:

  • Added a conditional macro definition for M_PI in eikonal.c. This ensures that M_PI is defined if it's not already provided by the environment or the included headers, specifically targeting compatibility with the MSVC compiler on Windows platforms.

Rationale:

The MSVC compiler, unlike some other compilers, does not define M_PI by default. This lack of definition caused build failures on Windows. By adding a conditional definition, we ensure broader compatibility and a smoother build process on Windows systems, especially for those using MSVC. Testing:

The modifications have been tested on a Windows 11 x64 23H2 system with MSVC 143 as the build tool. After applying these changes, the project builds successfully without any M_PI related errors.

This contribution aims to enhance the project's portability and ease of build on Windows platforms, potentially benefiting other Windows users who may face similar compilation issues.

You can view, comment on, or merge this pull request online at:

https://github.com/aaspip/pyekfmm/pull/5 Commit Summary

File Changes

(2 files https://github.com/aaspip/pyekfmm/pull/5/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/aaspip/pyekfmm/pull/5, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADHCWEWK776W3TYLJ53WBATYXPQ33AVCNFSM6AAAAABEOVCYA2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGE3TONJSGQ2TANQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>