Melt windows fix #137
Merged
Melt windows fix #137
Conversation
This file contains 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
|
Discussion:
|
TomMelt
force-pushed
the
melt-windows-fix
branch
from
e9db293 to
36cad91
Compare
TomMelt
force-pushed
the
melt-windows-fix
branch
from
36cad91 to
476d626
Compare
TomMelt
force-pushed
the
melt-windows-fix
branch
from
476d626 to
7856a33
Compare
jwallwork23
requested changes
Nov 19, 2024
jatkinson1000
requested changes
Nov 19, 2024
thanks. Just done in d64358c |
jwallwork23
approved these changes
Nov 21, 2024
There was a problem hiding this comment.
Thanks for adding the docs, @TomMelt. Just one minor request then good to go!
TomMelt
force-pushed
the
melt-windows-fix
branch
from
594e938 to
4d66327
Compare
jatkinson1000
approved these changes
Dec 9, 2024
There was a problem hiding this comment.
Couple of suggestions on phrasing, but not blocking.
Should be OK with this being added once #168 is approved and merged
Merged
TomMelt
force-pushed
the
melt-windows-fix
branch
from
8afc2d0 to
b233133
Compare
Integer size for FTorch can now be controlled using parameter `ftorch_int`. Currently, the default is set to `int32` but this could be changed if required. This fixes an issue on windows with the latest version of ifx/ifort which builds with a different integer type leading to the following error: ``` error #6284: There is no matching specific function for this generic function reference. [TORCH_TENSOR_FROM_ARRAY] in_tensors(1) = torch_tensor_from_array(in_data, tensor_layout, torch_kCPU) -------------------^ ```
TomMelt
force-pushed
the
melt-windows-fix
branch
from
b233133 to
d3d8865
Compare
fixes [#183](#183) There is an issue when building on mac (arm_64) or windows. The version of `libtorch` exposes a torch tensors shape (`t->sizes().data()`) as a `const long long int*` instead of just a `const long int*` like on linux and mac (x86). This commit adds preprocessor macro to switch between implementations automatically detecting the correct version at CMake build stage.
Co-authored-by: Jack Atkinson <[email protected]>
TomMelt
force-pushed
the
melt-windows-fix
branch
from
d3d8865 to
e82816e
Compare
|
the only test that fails is in the static analysis when running |
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.
fixes #124
Windows was not previously exporting the library symbols and creating the
dllcorrectly.This is now resolved by the following addition to
CMakeLists.txtFTorch/src/CMakeLists.txt
Lines 6 to 9 in 7856a33
Furthemore, there was an issue that the library was built with a different integer size than that used when compiling example programs, despite no difference in compiler option. We have now forced the library to use
int32explicitly. This can be modified by the user, modifying the following lineFTorch/src/ftorch.fypp
Line 34 in 7856a33
Tasks:
int64/int32issue properly (we have decided to build int32 by default. Can be overridden by user.