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

MAINT: ap.c gcc optimization portability #31

Open
tylerjereddy opened this issue Oct 21, 2020 · 2 comments
Open

MAINT: ap.c gcc optimization portability #31

tylerjereddy opened this issue Oct 21, 2020 · 2 comments

Comments

@tylerjereddy
Copy link
Member

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
Copy link
Member

cc @mtiberti — do you have any insights here?

@IAlibay IAlibay transferred this issue from MDAnalysis/mdanalysis Sep 6, 2023
@mtiberti
Copy link

same as #28

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

No branches or pull requests

3 participants