BUG, BLD: test_clustering_three_ensembles_two_identical fails on M1 Mac

[tylerjereddy]

Potentially related to #38, but there was a request in MDAnalysis/mdanalysis#3595 to open a separate issue for the M1 mac case (using gcc compile farm M1 machine gcc304, Python 3.8). In particular, without changing the build options on the M1, if I do:

• python3 -m pip install --user .
• python3 -m pytest MDAnalysisTests/analysis/test_encore.py::TestEncoreClustering::test_clustering_three_ensembles_two_identical

I get the traceback below the fold

========================================================== FAILURES ==========================================================
_____________________________ TestEncoreClustering.test_clustering_three_ensembles_two_identical _____________________________
self = <MDAnalysisTests.analysis.test_encore.TestEncoreClustering object at 0x1335e11f0>, ens1 = <Universe with 3341 atoms>
ens2 = <Universe with 3341 atoms>

    def test_clustering_three_ensembles_two_identical(self, ens1, ens2):
        cluster_collection = encore.cluster([ens1, ens2, ens1])
        expected_value = 40
>       assert len(cluster_collection) == expected_value, "Unexpected result:" \
                                                          " {0}".format(cluster_collection)
E       AssertionError: Unexpected result: <ClusterCollection with 42 clusters>
E       assert 42 == 40
E        +  where 42 = len(<ClusterCollection with 42 clusters>)

MDAnalysisTests/analysis/test_encore.py:450: AssertionError

Now, since we already have another issue open for this test on other architectures, and because there was already a setup.py shim in place for encore, I tried activating that compiler optimization adjustment on the M1 as well:

diff --git a/package/setup.py b/package/setup.py
index 5ba5678db..c14b03479 100755
--- a/package/setup.py
+++ b/package/setup.py
@@ -284,7 +284,7 @@ def extensions(config):
     # to avoid reducing optimisations on everything, we make a set of compile
     # args specific to encore see MDAnalysis/mdanalysis#2997 for an example of this.
     encore_compile_args = [a for a in extra_compile_args if 'O3' not in a]
-    if platform.machine() == 'aarch64' or platform.machine() == 'ppc64le':
+    if platform.machine() in ['aarch64', 'arm64', 'ppc64le']:
         encore_compile_args.append('-O1')
     else:
         encore_compile_args.append('-O3')

Repeating the same sequence of build/test commands I get a new/more serious looking traceback, below the fold

=========================================================== ERRORS ===========================================================
__________________________________ ERROR collecting MDAnalysisTests/analysis/test_encore.py __________________________________
ImportError while importing test module '/Users/treddy/github_projects/mdanalysis/testsuite/MDAnalysisTests/analysis/test_encore.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
MDAnalysisTests/analysis/test_encore.py:24: in <module>
    import MDAnalysis.analysis.encore as encore
../../../Library/Python/3.8/lib/python/site-packages/MDAnalysis/analysis/encore/__init__.py:23: in <module>
    from .similarity import hes, ces, dres, \
../../../Library/Python/3.8/lib/python/site-packages/MDAnalysis/analysis/encore/similarity.py:187: in <module>
    from .clustering.cluster import cluster
../../../Library/Python/3.8/lib/python/site-packages/MDAnalysis/analysis/encore/clustering/__init__.py:23: in <module>
    from . import ClusteringMethod
../../../Library/Python/3.8/lib/python/site-packages/MDAnalysis/analysis/encore/clustering/ClusteringMethod.py:41: in <module>
    from . import affinityprop
E   ImportError: dlopen(/Users/treddy/Library/Python/3.8/lib/python/site-packages/MDAnalysis/analysis/encore/clustering/affinityprop.cpython-38-darwin.so, 0x0002): symbol not found in flat namespace '_sqmIndex'
================================================== short test summary info ===================================================
ERROR MDAnalysisTests/analysis/test_encore.py
====================================================== 1 error in 4.42s ======================================================
ERROR: not found: /Users/treddy/github_projects/mdanalysis/testsuite/MDAnalysisTests/analysis/test_encore.py::TestEncoreClustering::test_clustering_three_ensembles_two_identical
(no name '/Users/treddy/github_projects/mdanalysis/testsuite/MDAnalysisTests/analysis/test_encore.py::TestEncoreClustering::test_clustering_three_ensembles_two_identical' in any of [<Module test_encore.py>])

What happens if I try a third option, -O2 optimization? I get the same traceback as at -03 above.
What happens if I try a fourth option, -O0 optimization? I get the same traceback as at -01 above.

What happens if I try a fifth option, -mcpu=apple-m1?

clang: error: the clang compiler does not support '-mcpu=apple-m1'

Ah, clang 12.0.5 is in use instead of a newer clang that supports it: https://stackoverflow.com/a/69926065/2942522
Would clang 13.x help here? Not sure, worth a try at some point, though the fact that we've had to adjust optimization level in the past for other archs suggests something is probably a bit more fragile than it should be.

We also need to be cautious of the other setup.py MacOS compiler injections like the part with:

if platform.system() == 'Darwin' and using_clang():

That said, only a single test (this one) seems fragile on MacOS M1, so things aren't that bad. The meson build system experiment seems to be going well for SciPy so maybe in a few months/years off-loading some of this maintenance burden to a well-maintained build-system will help, though we are pretty straightforward so far with building I think, which is nice.

[tylerjereddy]

I confirmed that the nature of the failure of this test is the same on Apple M2 Max with Apple clang version 14.0.3 and latest MDAnalysis develop at the time of writing (3ebd5ecfa09), so the xfail still needs to remain until we sort that out.

[mtiberti]

we'll be looking into whether this is a reasonable test that is appropriate for multiple architectures