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

Windows attempt #34

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open

Windows attempt #34

wants to merge 42 commits into from

Conversation

jaimergp
Copy link
Member

@jaimergp jaimergp commented May 16, 2020

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Strategies

Building on Windows will take a bit of effort... Amber team recommends the MinGW compilers, but some dependencies are built with other suites, which is specially complicated in the case of Fortran.

Option A: mys2

This is the current status for our first line of dependencies:

Dependency Windows Compiler Fortran support
cython Yes MSVC
numpy Yes MSVC
scipy Yes* MSVC/ifort Only through ifort! * Windows support happens via manual builds and requires MKL. After 2020.11.04, gfortran enabled builds are also available.
matplotlib Yes MSVC
bzip2 Yes MSVC
zlib Yes MSVC
boost-cpp Yes MSVC
libnetcdf Yes MSVC
netcdf-fortran NO! PR here
pthread-stubs Yes MinGW
fftw Yes MSVC NO! Previous attempt I might revisit
arpack Yes MinGW Yes
libblas/lapack/lapacke Yes MinGW Yes
readline NO!

As you can see, there's three main problems:

  1. netcdf-fortran: I submitted a PR to solve this, but we might need to wait for a new version upstream.
  2. fftw is on Windows but Fortran support is not enabled. I have raised an issue here.

Current status

Suboption A

Let CMake choose the bundled dependencies and build the troublesome deps. This includes boost, netcdf, netcdf-fortran, readline, fftw, and arpack. xblas static lib is being vendored. We might be able to distribute it like this if everything is made static.

Build

  • Dependencies are satisfied
  • Implemented script
  • CMake completes
  • Compilation ends successfully
    • FEW is not available because of some missing Perl libraries.

Test

  • Test imports
    • pytraj cannot be built on Windows+MinGW. Requires MSVC and maybe a separate packaging.
  • Test binaries
    • resp won't start
  • Test suite
    • run_test.bat
    • Provide m2-tcsh (done, recipe here)
    • Some binaries are called without the script extension (obviously, Unix-style), but in Windows these are provided with .bat or .exe. We need to provide temporary forwarders so the tests reach the executables.
    • Some input files cannot be found. This looks like a path-conversion issue.
    • parmed's bundled simtk.unit has a numpy C issue (missing DLL importing numpy.core._multiarray_umath).

Suboption B

In this case we use conda-forge libraries. Blocking:

  • netcdf-fortran is not available (yet) and compilation fails because of the same issues as in the PR. Once [WIP] Attempt windows netcdf-fortran-feedstock#52 is merged, we might get further. Interesting how this doesn't happen if netcdf is compiled at the same time.
  • arpack (lacks arsecond?)
  • fftw (no Fortran bindings)
  • boost (no compression support)
  • readline (not available).

Option B: MSVC + flang

With this approach, we would use MSVC compilers for C/C++ and flang for Fortran. Flang packages do not always work (scipy example), so it might be tricky.

Note: This might change in the future because (A) apparently ifort support is coming to CF, and (B) LLVM will provide full flang support.

Large parts of Amber only expect a MinGW-like compiler, so they won't be able to use another compilers unless the name mangling is done in a MinGW-compatible way.

Given the small margin for success here, I won't even try for now :)

@jaimergp jaimergp requested a review from simonbray as a code owner May 16, 2020 14:03
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

@jaimergp
Copy link
Member Author

@conda-forge-admin, please rerender

@jaimergp
Copy link
Member Author

@conda-forge-admin, please rerender

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [71, 73, 92, 94]

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

@jaimergp jaimergp changed the title Prepare for windows attempt Windows attempt May 17, 2020
@jaimergp
Copy link
Member Author

This last commit works (builds) but the tests do not.

@jaimergp
Copy link
Member Author

Managed to run the test suite locally, more or less! Some tests hang so you have to Ctrl+C to skip to the next (happens a couple of times).

These are the results:

Finished test suite for AmberTools at 21 May 2020 15:26:05.

make[1]: Leaving directory 'C:/Users/Jaime/Miniconda3/conda-bld/ambertools_1589972034265/test_tmp/ambertools/test'
809 file comparisons passed
63 file comparisons failed (0 of which can be ignored)
98 tests experienced errors
Test log file saved as /cygdrive/c/Users/Jaime/Miniconda3/conda-bld/ambertools_1589972034265/_test_env/Library/logs/test_at_serial/2020-05-21_12-35-42.log
Test diffs file saved as /cygdrive/c/Users/Jaime/Miniconda3/conda-bld/ambertools_1589972034265/_test_env/Library/logs/test_at_serial/2020-05-21_12-35-42.diff

Full logs and diffs:
2020-05-21_12-35-42.zip

@jaimergp
Copy link
Member Author

Yay, that made the test suite run! Now we can actually test on the CI, awesome! And on Azure there's not Ctrl+C issues, so the results are more "honest":

Finished test suite for AmberTools at Thu May 21 17:50:02 CUT 2020.
899 file comparisons passed
44 file comparisons failed (0 of which can be ignored)
109 tests experienced errors
Test log file saved as /cygdrive/d/bld/ambertools_1590072710262/_test_env/Library/logs/test_at_serial/2020-05-21_16-17-01.log
Test diffs file saved as /cygdrive/d/bld/ambertools_1590072710262/_test_env/Library/logs/test_at_serial/2020-05-21_16-17-01.diff

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

…nda-forge-pinning 2020.06.07.16.42.19 (windows only again)
@jaimergp
Copy link
Member Author

jaimergp commented Jun 9, 2020

Tests seem to get stuck at different parts:

cd fpph && ./Run.fpph_resp

Running locally to investigate why.

@jaimergp
Copy link
Member Author

jaimergp commented Jun 9, 2020

Ok, that "works" so at least we can run the full suite reliably now.

Summary for now:

Finished test suite for AmberTools at Tue Jun  9 15:38:55 CUT 2020.

925 file comparisons passed
48 file comparisons failed (0 of which can be ignored)
97 tests experienced errors

Detailed analysis coming soon.

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.

2 participants