Skip to content

[ampl-mp] Add new port#5331

Merged
vicroms merged 2 commits intomicrosoft:masterfrom
zhihaoy:ampl-mp
Feb 11, 2019
Merged

[ampl-mp] Add new port#5331
vicroms merged 2 commits intomicrosoft:masterfrom
zhihaoy:ampl-mp

Conversation

@zhihaoy
Copy link
Contributor

@zhihaoy zhihaoy commented Feb 8, 2019

This port aims for solver development, so it does not build any existing solver modules.

The shared library build is disabled for now because it has some problem on Windows.

There is a bug on the MSVC side, I've reported it to https://developercommunity.visualstudio.com/content/problem/441193/ice-when-optimizing-c-code.html , for now I disabled code optimization on the affected files.

@MVoz
Copy link
Contributor

MVoz commented Feb 8, 2019

LINK : fatal error
LNK1104: cannot open file 'dl.lib' [E:\tools\vcpkg\buildtrees\ampl-mp\x64-windows-static-dbg\src\asl\tableproxy.vcxproj]

cd vcpkg\buildtrees\ampl-mp\x64-windows-static-dbg cmake . -LA

DL_LIBRARY:FILEPATH=E:/tools/vcpkg/installed/x64-windows-static/debug/lib/dl.lib

tableproxy.vcxproj

  <AdditionalDependencies>..\..\lib\RelWithDebInfo\asl.lib;wsock32.lib;..\..\lib\RelWithDebInfo\mp.lib;dl.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib</AdditionalDependencies>
  <AdditionalLibraryDirectories>%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 8, 2019

Should have been fixed. @voskrese , can you give a check? This is something can be fixed upstream so I will propose a patch there as well.

@vicroms vicroms self-assigned this Feb 8, 2019
@MVoz
Copy link
Contributor

MVoz commented Feb 8, 2019

Yes, the problem was solved, but another one arose, with MATLAB

Microsoft (R) C/C++ версии 19.16.27026.1 для x64
MS SDK \ Windows Target Platform Version 10.0.17134.0

Windows Kits\8.1\include ?

...
C:\Program Files (x86)\Windows Kits\8.1\\include\um\winnt.h(396): error C2061: синтаксическая ошибка: идентификатор "WC
HAR" [E:\tools\vcpkg\buildtrees\ampl-mp\x64-windows-static-rel\src\asl\spamfunc.vcxproj]
...

or more error

workaround, comment out matlab

if (NOT WIN32)
find_package(MATLAB)
if (MATLAB_FOUND)
  set(matlab_asl asl)
  if (MSVC)
....
  endforeach ()
endif ()
endif ()

Build spamfunc.mex

ampl/mp@0e18391?diff=split

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 8, 2019

Is that caused by an SDK difference? I don't have a MATLAB to test against. This may be desirable by some but let me disable it for now.

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 8, 2019

The local patch to fix the DL issue has been accepted in upstream, so "rebased" the ports version.

@MVoz
Copy link
Contributor

MVoz commented Feb 9, 2019

to the MATLAB, I want to note that the error occurs when compiling with the option -DBUILD=asl , if the assembly is made with the option -DBUILD=all, then there is no such error, but many others arise, I have not found a ready solution to eliminate them. Multiple solutions, disable drivers
-DHAVE_ACCESS_DRIVER_EXITCODE=0 -DHAVE_EXCEL_DRIVER_EXITCODE=0 -DHAVE_ODBC_TEXT_DRIVER_EXITCODE=0

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 9, 2019

to the MATLAB, I want to note that the error occurs when compiling with the option -DBUILD=asl , [...] Multiple solutions, [...]

I thank you on behave of the future people/team who are care enough to work on this part :) As written in the description of this pull request, I think the port is good as long as it supports solver development well.

This port aims for solver development, so it does not build any
existing solver modules.

The shared library build is disabled for now because it has some
problem on Windows.

closes: microsoft#4491
@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 11, 2019

Ping @vicroms can we get this tested by today? Thanks.

@vicroms
Copy link
Member

vicroms commented Feb 11, 2019

I just triggered a CI build

@vicroms
Copy link
Member

vicroms commented Feb 11, 2019

Here are the build results:

Platform Result
arm-uwp fail
arm64-windows fail
x64-linux pass
x64-osx pass
x64-upw fail
x64-windows pass
x64-windows-static pass
x86-windows pass

I think this looks good for an initial port, so I'll go ahead and merge the PR.
Please open a new PR if you want to make further improvements to the port.

@vicroms vicroms merged commit e6b174e into microsoft:master Feb 11, 2019
@zhihaoy
Copy link
Contributor Author

zhihaoy commented Feb 11, 2019

arm-uwp fail
arm64-windows fail
x64-uwp fail

I think this looks good for an initial port, so I'll go ahead and merge the PR.
Please open a new PR if you want to make further improvements to the port.

Thanks. Quick question, what are the minimal components I need to install in VS to test these?

@vicroms
Copy link
Member

vicroms commented Feb 14, 2019

@zhihaoy

I think these are the relevant components in VS:

  • Visual C++ compilers and libraries for ARM
  • Visual C++ compilers and libraries for ARM64

For UWP, I installed the Universal Windows Platform development workload, but maybe just adding these Individual Component should be enough:

  • Visual C++ runtime for UWP
  • C++ Universal WIndows Platform tools for ARM64

And maybe:

  • Windows 10 SDK

@JackBoosY
Copy link
Contributor

JackBoosY commented Nov 10, 2020

@zhihaoy ampl-mp deleted the target asl in the latest master (ampl/mp@dcf32ab#diff-f48835fa51cb9f3658c3f012f53afb8b834b3bcc5ce6334a15b0461277657b6e), should we change the repo?

And I want to confirm whether the target of this port is mp or asl?

@zhihaoy
Copy link
Contributor Author

zhihaoy commented Nov 10, 2020

@zhihaoy ampl-mp deleted the target asl in the latest master (ampl/mp@dcf32ab#diff-f48835fa51cb9f3658c3f012f53afb8b834b3bcc5ce6334a15b0461277657b6e), should we change the repo?

And I want to confirm whether the target of this port is mp or asl?

This port used to install both targets. The main target is mp. Removing the asl library probably simplified this library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants