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

Add PyAlbany #647

Merged
merged 4 commits into from
Dec 17, 2020
Merged

Add PyAlbany #647

merged 4 commits into from
Dec 17, 2020

Conversation

kliegeois
Copy link
Contributor

@kliegeois kliegeois commented Dec 10, 2020

This PR includes a python interface for Albany: PyAlbany.

The original goal of this interface was to ease the usage of reduced Hessian-vector products by providing ways to read and write distributed multivectors.

PyAlbany is not enabled by default and none of the work included in this PR should impact existing tests or builds.

PyAlbany is built against PyTrilinos, the python interface of Trilinos.
Currently, it is required to list both the source directory and the build directory of Trilinos to compile PyAlbany correctly.
It will be possible to avoid this in the future if PyTrilinos installs some swig files.

A detailed explanation of the building process and the dependencies can be found in this ReadMe.

This PR includes tests and an example.
The code has been tested with Python 2 and 3 (note that Python 3 build needs a small modification of Trilinos which is not merged yet PR #8444); in debug and release (note that PyAlbany cannot be compiled warning-free; I did not achieve to build PyTrilinos warning-free).

I tried to be verbose on the comments and documentation.

@mperego @ikalash @bartgol @jewatkins

@bartgol
Copy link
Collaborator

bartgol commented Dec 10, 2020

What if we ignored python2 completely and require python3?

@kliegeois
Copy link
Contributor Author

@bartgol it will mainly simplify the cmake files. Except that, I don't think that it will impact the other files.

Copy link
Collaborator

@mperego mperego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great! However, I get a seg fault when I try to run the tests

-D Trilinos_ENABLE_PyTrilinos:BOOL=ON \
-D PyTrilinos_DOCSTRINGS:BOOL=OFF \
```
The first option enable PyTrilinos and the second one disables PyTrilinos docstrings. This last option is required if you are using Doxygen 1.8.13.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add also
-D SWIG_EXECUTABLE:FILEPATH="${HOME}/Workspace/TPL/swig/swig-3.0.11/install/bin/swig"
both in the Trilinos and in the Albany cmake script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your inputs about the README.md, I updated it to include them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but -D SWIG_EXECUTABLE="/usr/bin/swig" \ needs to be added to the Albany config as well.

@mperego
Copy link
Collaborator

mperego commented Dec 16, 2020

All the 4 PyAlbany tests fail in a similar way:
LastTest.log

@kliegeois
Copy link
Contributor Author

All the 4 PyAlbany tests fail in a similar way:
LastTest.log

@mperego thanks a lot for having tested them.

There is definitely something wrong.

I wonder whether the issue could be related to the version of mpi used by mpi4py.

Could you test the following first?

  1. Reconfigure and build Trilinos with:

    -D PyTrilinos_ENABLE_TESTS:BOOL=ON \
    

    (this should not require any extra compilation as the tests are basically written in python).

  2. And run:

    ctest -R PyTrilinos
    

    In particular, does the test PyTrilinos_testTeuchos_Comm_MPI_4 passed?

    1. If PyTrilinos_testTeuchos_Comm_MPI_4 failed, the issue is not directly related to PyAlbany itself.
      It is probably related to mpi4py which might be using a different version of mpi than the one used by Trilinos.
      Can you verify if the mpi4py .so is linked to the version of mai used to compile Trilinos/Albany?
      To do so, start by locating the MPI.so file as follows:

      -bash-4.2$ python
      >>> import mpi4py as MPI
      >>> print(MPI.__file__)
      

      this will print something such as: /ascldap/users/knliege/local/python_blake/lib/python2.7/site-packages/mpi4py/__init__.pyc

      And, check the mpi lib as follows (based on the output of the previous script):

      ldd /ascldap/users/knliege/local/python_blake/lib/python2.7/site-packages/mpi4py/MPI.so | grep "libmpi"
      

      Do the same with a Trilinos library such as libteuchoscore.so:

      ldd /ascldap/users/knliege/local/trilinos_all/blake_Albany/lib/libteuchoscore.so | grep "libmpi"
      

      Finally, verify that mpirun is consistent with both of those libs.

    2. If PyTrilinos_testTeuchos_Comm_MPI_4 passed, can you test the following and check if it is failing when importing wpyalbany?

      -bash-4.2$ python
      >>> from PyTrilinos import Tpetra
      >>> from PyTrilinos import Teuchos
      >>> from PyAlbany import wpyalbany as wpa
      

@kliegeois
Copy link
Contributor Author

Thanks @mperego for your review, I updated the files as suggested.

@mperego
Copy link
Collaborator

mperego commented Dec 16, 2020

@kliegeois Only this PyTrilinos test fail:
PyTrilinos_testEpetraExt_HDF5_MPI_4

Here's the path of mpi4py

>>> import mpi4py as MPI
>>> print(MPI.__file__)
/ascldap/users/mperego/anaconda3/lib/python3.8/site-packages/mpi4py/__init__.py

It can import Tpetra and Teuchos but not wpyalbany

from PyAlbany import wpyalbany as wpa
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'wpyalbany' from 'PyAlbany' (/ascldap/users/mperego/Workspace/albany/sources/albany-src/PyAlbany/PyAlbany/__init__.py)

Copy link
Collaborator

@mperego mperego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good!

@kliegeois
Copy link
Contributor Author

Thanks @mperego ! I will update the README.md tomorrow and, at least to me, the PR will be good to be merged.

@kliegeois
Copy link
Contributor Author

@mperego I have just updated the README.md (and squashed the related commits).
Could you let me know if the precision is fine for you?

@mperego
Copy link
Collaborator

mperego commented Dec 17, 2020

@kliegeois Looks good to me. Thanks. Feel free to go ahead and merge the branch.

@kliegeois
Copy link
Contributor Author

@mperego thanks!

Well, actually, it seems that the access that I have received is not sufficient to merge the PR:

Capture d’écran 2020-12-17 à 15 59 46

@ikalash would you agree to give me the required write access?

@mperego
Copy link
Collaborator

mperego commented Dec 17, 2020

@mperego thanks!

Well, actually, it seems that the access that I have received is not sufficient to merge the PR:

Capture d’écran 2020-12-17 à 15 59 46

@ikalash would you agree to give me the required write access?

I'll merge this PR for you

@mperego mperego merged commit 09b8ea0 into sandialabs:master Dec 17, 2020
@kliegeois kliegeois deleted the pyalbany branch December 17, 2020 15:11
@kliegeois
Copy link
Contributor Author

Thanks @mperego !

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.

3 participants