MDAnalysis / mdaencore

Ensemble overlap comparison software for molecular data.
http://www.mdanalysis.org/mdaencore/
GNU General Public License v2.0
0 stars 0 forks source link

MAINT: `ap.c` gcc optimization portability #31

Open tylerjereddy opened 3 years ago

tylerjereddy commented 3 years ago

For the source file located at path analysis/encore/clustering/src/ap.c, the gcc compile optimization level can affect test results in the test_encore.py suite on the Linux ARM64 platform.

At -O1, where we are currently pinned, we get a pass on the suite.

At -O2 and -O3 we get an incorrect result:

_________________________________ TestEncoreClustering.test_clustering_three_ensembles_two_identical _________________________________

self = <MDAnalysisTests.analysis.test_encore.TestEncoreClustering object at 0x7f5a74b750>, 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:448: AssertionError

I carefully checked the values of C-level constants like RAND_MAX and the sizes of types used in the source file via sizeof() and found no differences whatsoever between ARM64 and AMD64.

Perhaps this set of observations will be helpful to the low-level gurus for determination of changes needed to preserve performance while improving portability.

orbeckst commented 3 years ago

cc @mtiberti — do you have any insights here?

mtiberti commented 10 months ago

same as #28