aburrell commented 2 years ago

Description

Update Fortran source code in the MESON reorg branch.

Type of change

Please delete options that are not relevant.

Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Running tests locally

Test Configuration:

Operating system: OS X Big Sur
Python version number: 3.9
Compiler with version number: gfortran 11.3.0
Any details about your local setup that are relevant (e.g., apexpy version/branch)

Checklist:

[N/A] Make sure you are merging into the develop (not main) branch
[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[x] Any dependent changes have been merged and published in downstream modules
[x] Add a note to Changelog.rst, summarising the changes
[x] Add yourself to AUTHORS.rst and .zenodo.json

aburrell commented 2 years ago

Made changes following advice from https://github.com/mesonbuild/meson/issues/10536.

aburrell commented 2 years ago

@ljlamarche can you take a look at the meson.build file and let me know if you can find anything wrong? I'm following the examples from:

https://numpy.org/doc/stable/f2py/buildtools/meson.html https://github.com/scipy/scipy/blob/55fca818b49d5730c3cc77d012a593a910f3c03c/scipy/meson.build#L52 https://github.com/scipy/scipy/pull/16475/files

I think the failure lies somewhere between the definition of fortranapex_source and the extension_module definition, but I am stumped.

ljlamarche commented 2 years ago

Sure! I'll take a look, but it might take me a few days to poke at this.

ljlamarche commented 2 years ago

When I run meson compile, I get the following error:

FAILED: fortranapex.cpython-39-darwin.so.p/depscan.dd 
/Users/e30737/miniconda3/envs/apexpy/bin/meson --internal depscan fortranapex.cpython-39-darwin.so.p/fortranapex.cpython-39-darwin.dat fortranapex.cpython-39-darwin.so.p/depscan.dd /Users/e30737/Desktop/Software/apexpy/builddir/fortranapex.cpython-39-darwin.so.p/fortranapex.cpython-39-darwin-deps.json
Traceback (most recent call last):
  File "/Users/e30737/miniconda3/envs/apexpy/bin/meson", line 10, in <module>
    sys.exit(main())
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/mesonmain.py", line 266, in main
    return run(sys.argv[1:], launcher)
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/mesonmain.py", line 255, in run
    return run_script_command(args[1], args[2:])
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/mesonmain.py", line 215, in run_script_command
    return module.run(script_args)
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/scripts/depscan.py", line 208, in run
    return scanner.scan()
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/scripts/depscan.py", line 158, in scan
    self.scan_file(s)
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/scripts/depscan.py", line 58, in scan_file
    self.scan_fortran_file(fname)
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/site-packages/mesonbuild/scripts/depscan.py", line 67, in scan_fortran_file
    for line in fpath.read_text(encoding='utf-8', errors='ignore').split('\n'):
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/pathlib.py", line 1266, in read_text
    with self.open(mode='r', encoding=encoding, errors=errors) as f:
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/pathlib.py", line 1252, in open
    return io.open(self, mode, buffering, encoding, errors, newline,
  File "/Users/e30737/miniconda3/envs/apexpy/lib/python3.9/pathlib.py", line 1120, in _opener
    return self._accessor.open(self, flags, mode)
FileNotFoundError: [Errno 2] No such file or directory: 'fortranapex-f2pywrappers.f90'
[6/14] Compiling C object fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapexmodule.c.o
ninja: build stopped: subcommand failed.

Is that the same problem you're getting?

aburrell commented 2 years ago

Yes it is!

rgommers commented 2 years ago

I'll note that <modulename>-f2pywrappers.f90 isn't always generated, it depends on the content of the Fortran and .pyf files. That is pretty annoying, I know. You can check if it's generated under build/apexpy. Maybe it's not there at all, or with a different extension, or one level deeper in the directory tree than you expect. Regarding the last point, it's typically better to have hierarchical meson.build files with extension modules defined in the subdir they need to be installed to, rather than everything in a single top-level meson.build file.

ljlamarche commented 2 years ago

I do see fortranapex-f2pywrappers2.f90 in my build directory. I'm not sure if this is significant or not, but it doesn't complain about fortranapexmodule.c and that's in the same build directory.

ljlamarche commented 2 years ago

Oh wait, I see. It's supposed to make fortranapex-f2pywrappers.f90 but instead it's making fortranapex-f2pywrappers2.f90.

aburrell commented 1 year ago

So, it seems to be making fortranapex-f2pywrappers2.f90 because it is also making an empty fortranapex-f2pywrappers.f. So, I updated the name (assuming that this will be a consistent naming convention for a while at least) and it gets past that step. Now it fails with this error:

 [6/8] Compiling Fortran object fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapex-f2pywrappers2.f90.o
      FAILED: fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapex-f2pywrappers2.f90.o
      gfortran -Ifortranapex.cpython-39-darwin.so.p -I. -I../.. -I/private/var/folders/d5/d42l5vcd5pq112_vffrvs3w00005c0/T/pip-build-env-iaap4m20/overlay/lib/python3.9/site-packages/numpy/core/include -I/private/var/folders/d5/d42l5vcd5pq112_vffrvs3w00005c0/T/pip-build-env-iaap4m20/overlay/lib/python3.9/site-packages/numpy/f2py/src -Ilib_fortranobject.a.p -I/opt/local/Library/Frameworks/Python.framework/Versions/3.9/include/python3.9 -fvisibility=hidden -fdiagnostics-color=always -Wall -std=legacy -O2 -Wno-conversion -Jfortranapex.cpython-39-darwin.so.p -o fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapex-f2pywrappers2.f90.o -c fortranapex-f2pywrappers2.f90
      fortranapex-f2pywrappers2.f90:7:11:

          7 |       use magcof, only : nmax1
            |           1
      Fatal Error: Cannot open module file 'magcof.mod' for reading at (1): No such file or directory
      compilation terminated.

I also tried going in and compiling the fortran without cleaning up the files so that 'magcof.mod' exists, but I get the exact same error.

I also tried making hierarchical meson.build files and got the exact same error.

aburrell commented 1 year ago

Nevermind! I found what I did wrong!

aburrell commented 1 year ago

Ok, so now it looks like it installs correctly:

% pip install . --user       
Processing /Users/aburrell/Programs/Git/apexpy
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: numpy>=1.19.5 in /opt/local/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages (from apexpy==1.1.0) (1.23.0)
Building wheels for collected packages: apexpy
  Building wheel for apexpy (pyproject.toml) ... done
  Created wheel for apexpy: filename=apexpy-1.1.0-cp39-cp39-macosx_11_7_x86_64.whl size=262263 sha256=ca7c67cd38f6d9cb8c0629b6d8cb543829e03543396ec2bd967c0ce2d96d1c37
  Stored in directory: /private/var/folders/d5/d42l5vcd5pq112_vffrvs3w00005c0/T/pip-ephem-wheel-cache-p2apgmgi/wheels/0a/5f/b4/e52af379d195bc93961354244d82b1be91dde4855df03f9759
Successfully built apexpy
Installing collected packages: apexpy
  Attempting uninstall: apexpy
    Found existing installation: apexpy 1.1.0
    Uninstalling apexpy-1.1.0:
      Successfully uninstalled apexpy-1.1.0
Successfully installed apexpy-1.1.0

But it doesn't load the fortran library. The error looks like:

In [1]: import apexpy
fortranapex module could not be imported. apexpy probably won't work. Failed with error: dlopen(/Users/aburrell/Library/Python/3.9/lib/python/site-packages/apexpy/fortranapex.cpython-39-darwin.so, 2): Library not loaded: @rpath/libgfortran.5.dylib
  Referenced from: /Users/aburrell/Library/Python/3.9/lib/python/site-packages/apexpy/fortranapex.cpython-39-darwin.so
  Reason: no suitable image found.  Did find:
    /Users/aburrell/lib/libgfortran.5.dylib: stat() failed with errno=20

The .so file exists in the correct location. Not sure why it won't load. The "not a directory" error (errno=20) is happening because /Users/aburrell/lib is a binary file and not a directory. Not sure why Python is looking there.

rgommers commented 1 year ago

That is quite odd indeed. How did you install gfortran 11.3.0, Homebrew? And can you check where libgfortran should actually be loaded from, with locate libgfortran.5.dylib?

It's possible it's the --user flag. Could you install build and then build the wheel with python -m build . and then install it with pip install dist/*.whl and see if the problem goes away?

ljlamarche commented 1 year ago

So I'm actually still having problems with meson compile. That fails with a linking error:

FAILED: fortranapex.cpython-39-darwin.so 
cc  -o fortranapex.cpython-39-darwin.so fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapexmodule.c.o fortranapex.cpython-39-darwin.so.p/meson-generated_.._fortranapex-f2pywrappers2.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_magfld.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_apex.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_makeapexsh.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_igrf.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_apexsh.f90.o fortranapex.cpython-39-darwin.so.p/fortranapex_checkapexsh.f90.o fortranapex.cpython-39-darwin.so.p/_Users_e30737_miniconda3_envs_apexpy_lib_python3.9_site-packages_numpy_f2py_src_fortranobject.c.o -L/usr/local/lib/gcc/aarch64-apple-darwin20/11.0.0 -L/usr/local/lib/gcc/aarch64-apple-darwin20/11.0.0/../../.. -L/usr/local/lib -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,dynamic_lookup -bundle -Wl,-undefined,dynamic_lookup -lm -lquadmath lib_fortranobject.a -lgfortran
ld: library not found for -lquadmath
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Any ideas, or am I approaching this wrong?

aburrell commented 1 year ago

I encountered that error earlier and it was fixed by adding a flag: add_project_link_arguments('-lquadmath', language: ['c', 'fortran']). You might need to re-compare or re-pull the branch @ljlamarche (though I'm surprised you'd be that far back).

ljlamarche commented 1 year ago

I literally just pulled and my meson.build file does contain that addition (line 46, right?)... Odd. Maybe it's something with how my fortran libraries are set up? I'll poke at this locally.

aburrell commented 1 year ago

I literally just pulled and my meson.build file does contain that addition (line 46, right?)... Odd. Maybe it's something with how my fortran libraries are set up? I'll poke at this locally.

Yeah, would be shocked if you hadn't. Maybe the code is cursed? 👻

On a more helpful note, maybe try playing around with the order of the fortran source files. That's been an issue in the SuperDARN RST code before.

aburrell commented 1 year ago

I use MacPorts rather than HomeBrew. I've got:

/opt/local/lib/gcc10/libgfortran.5.dylib
/opt/local/lib/gcc11/libgfortran.5.dylib
/opt/local/lib/gcc9/libgfortran.5.dylib
/opt/local/lib/libgcc/libgfortran.5.dylib

However, the CI is failing in the same way, so I don't think the problem is specific to my set up.

I built the wheel successfully, but then pip puked by not recognizing it. I'm updating everything at the moment to see if that helps. So... a combination of different set up problems potentially.

rgommers commented 1 year ago

Ah wait, this rings a bell. It may be https://github.com/mesonbuild/meson/issues/10711. I haven't seen that yet with conda-forge, where I'm on gfortran 10.4.0. I will try to see if I can replicate your issue.

rgommers commented 1 year ago

This PR builds and imports fine on both Linux and macOS with conda-forge compilers. And then I installed gfortran 12.2.0 from Homebrew - also builds and imports fine. Checking rpath on the installed extension module:

otool -L fortranapex.cpython-39-darwin.so
fortranapex.cpython-39-darwin.so:
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)
    /opt/homebrew/opt/gcc/lib/gcc/current/libgfortran.5.dylib (compatibility version 6.0.0, current version 6.0.0)

I don't use MacPorts, so not easy to check. Maybe you can try with either Homebrew or conda-forge?

The CI failures are not the same issue, the problem looks like the test command:

coverage run --rcfile=setup.cfg -m pytest

There's an issue with pytest here too - it's a real pain to run tests against an install version of your package, unless you have installed the tests too. I can reproduce the CI problem showing

E   ImportError: cannot import name 'fortranapex' from 'apexpy' (/home/rgommers/code/tmp/apexpy/apexpy/__init__.py)

while the actual import from apexpy import fortranapex as fa works perfectly fine.

aburrell commented 1 year ago

Ok, it seems I may be having trouble because I can't get meson to use gcc in place of clang using python -m build . despite creating a .mesonpy-native-file.ini file and making sure my environment variables are correctly set.

rgommers commented 1 year ago

Ok, it seems I may be having trouble because I can't get meson to use gcc in place of clang using python -m build . despite creating a .mesonpy-native-file.ini file and making sure my environment variables are correctly set.

Setting the CC, CXX and FC environment variables to the compilers you want is enough to do explicit compiler selection.

ljlamarche commented 1 year ago

I literally just pulled and my meson.build file does contain that addition (line 46, right?)... Odd. Maybe it's something with how my fortran libraries are set up? I'll poke at this locally.

Yeah, would be shocked if you hadn't. Maybe the code is cursed? 👻

On a more helpful note, maybe try playing around with the order of the fortran source files. That's been an issue in the SuperDARN RST code before.

Haha, mistakes happen, always good to double check. No, I think my problem is related to the fact that I don't seem to have the libquadmath library on my system AT ALL, despite trying to obtain gfortran from a couple of different sources. I assume this is one of the classic "new mac" problems. I'll let you know when I figure it out!

rgommers commented 1 year ago

If it helps, I see libquadmath just fine for both the conda-forge and the Homebrew builds of gfortran:

locate libquadmath
/Users/rgommers/mambaforge/envs/scipy-dev/lib/libquadmath.0.dylib
/Users/rgommers/mambaforge/envs/scipy-dev/lib/libquadmath.dylib
/Users/rgommers/mambaforge/pkgs/libgfortran5-11.3.0-hdaf2cc0_25/lib/libquadmath.0.dylib
/Users/rgommers/mambaforge/pkgs/libgfortran5-11.3.0-hdaf2cc0_25/lib/libquadmath.dylib
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/12/libquadmath.0.dylib
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/12/libquadmath.a
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/12/libquadmath.dylib
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/current/libquadmath.0.dylib
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/current/libquadmath.a
/opt/homebrew/Cellar/gcc/12.2.0/lib/gcc/current/libquadmath.dylib

aburrell commented 1 year ago

Ok, I got things to install locally and run tests locally. I made a note of the issues I encountered in the docs. To get coverage to work locally I had to: 1) remove apexpy/tests/__init__.py 2) remove the coverage source specification in setup.cfg

Name                                                                                 Stmts   Miss Branch BrPart     Cover   Missing
-----------------------------------------------------------------------------------------------------------------------------------
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/apexpy/__init__.py           9      2      0      0    77.78%   6-7
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/apexpy/apex.py             282      2    112      2    98.98%   14-15, 507->527, 517->527
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/apexpy/helpers.py           70      0     12      0   100.00%
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/pytest_cov/__init__.py       1      0      0      0   100.00%
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/pytest_cov/compat.py        21      7      6      1    62.96%   14, 19-23, 27, 31
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/pytest_cov/embed.py         80     62     28      0    16.67%   25-28, 33-34, 44-79, 83-90, 98-105, 117-129, 133-136, 140
/Users/aburrell/Library/Python/3.9/lib/python/site-packages/pytest_cov/plugin.py       219    154     92      2    26.05%   32-54, 58-68, 72-76, 81-82, 133, 141-146, 149-150, 170-200, 206-227, 230, 235-254, 261-262, 270-271, 275, 278-279, 285-313, 316-344, 347-350, 353, 357-363, 368, 371, 374, 377, 380-382, 388, 396-400
apexpy/tests/test_Apex.py                                                              673      4    107      2    99.23%   42->48, 46-47, 51->57, 55-56
apexpy/tests/test_cmd.py                                                                83      1     14      3    95.88%   19->23, 35->38, 74
apexpy/tests/test_fortranapex.py                                                        74      0      4      0   100.00%
apexpy/tests/test_helpers.py                                                           107      0      8      0   100.00%
-----------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                 1619    232    383     10    82.72%

So, progress! I got to tick the "tests pass locally box". The CI issues that remain are:

Windows installation fails
Mac/Linux coverage call tries to import local apexpy instead of using installed version.
Codacy failing, need to check that the build commands there are correct.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3362689961

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+4.5%) to 98.839%

Totals
Change from base Build 2630780515:	4.5%
Covered Lines:	357
Relevant Lines:	361

💛 - Coveralls

ljlamarche commented 1 year ago

FYI the build instructions aren't working for me:

(apexpy) e30737@Lamarchel-mbp16 apexpy % python -m build .
/Users/e30737/miniconda3/envs/apexpy/bin/python: No module named build.__main__; 'build' is a package and cannot be directly executed

aburrell commented 1 year ago

Ok @ljlamarche I do not know how to make coveralls or coverage work. If you could poke at that (maybe in the coverage options in setup.cfg) that would be great. Or we could try and move coverage to codacy.

ljlamarche commented 1 year ago

@aburrell Deal, I'll take a look at this as soon as I get my install working again. Full disclosure, I've never used coveralls, but maybe a fresh set of eyes?

ljlamarche commented 1 year ago

Ok, I somehow didn't have xcode installed... Oh well, pytest pass locally now. I'll start looking at coverage.

aburrell commented 1 year ago

@ljlamarche I think I fixed it!

aburrell / apexpy

Fortran reorg #93

Description

Type of change

How Has This Been Tested?

Checklist:

Pull Request Test Coverage Report for Build 3362689961

💛 - Coveralls