PrincetonUniversity / SPEC

The Stepped-Pressure Equilibrium Code, an advanced MRxMHD equilibrium solver.
https://princetonuniversity.github.io/SPEC/
GNU General Public License v3.0
24 stars 4 forks source link

SPEC python wrapper debugging #119

Closed zhucaoxiang closed 3 years ago

zhucaoxiang commented 3 years ago

Hi all, I have the python wrapper close to finish. For more details, you can check the pywrapper branch. There is readme documentation available at Utilities/python_wrapper/README.md.

There are some final bugs. I am wondering if any of you have any suggestions.

  1. Test on Mac with GCC-10. Compile successfully, but get the following error when importing the python package. I tried searching online but didn't resolve it.

    Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/Users/caoxiang/Documents/Codes/SPEC/Utilities/python_wrapper/spec.py", line 2, in <module>
    import _spec
    ImportError: dlopen(/Users/caoxiang/Documents/Codes/SPEC/Utilities/python_wrapper/_spec.cpython-38-darwin.so, 2): Symbol not found: ___allglobal_MOD_ajk
    Referenced from: /Users/caoxiang/Documents/Codes/SPEC/Utilities/python_wrapper/_spec.cpython-38-darwin.so
    Expected in: flat namespace
    in /Users/caoxiang/Documents/Codes/SPEC/Utilities/python_wrapper/_spec.cpython-38-darwin.so
    make: *** [test] Error 1
  2. Test on PPPL cluster with GCC-9.3.0 Get the following error when importing. Have no clue what was happening.

    Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/p/focus/share/spec/Utilities/python_wrapper/spec.py", line 2, in <module>
    import _spec
    ImportError: /p/focus/share/spec/Utilities/python_wrapper/_spec.cpython-37m-x86_64-linux-gnu.so: undefined symbol: __allglobal_MOD_lmavalue
    make: *** [test] Error 1
  3. Github actions error (solved) Get an error in the Github actions while the master branch works fine. More details can be found at https://github.com/PrincetonUniversity/SPEC/runs/1413458507?check_suite_focus=true

zhucaoxiang commented 3 years ago

The third issue is solved and was caused by the macro line replacing CHARACTER with character. Although I am not quite sure what is the real motivation under this line, it will convert CHARACTER(100) :: ext into character :: ext and change the dimension.

zhucaoxiang commented 3 years ago

The bugs are now resolved. Thanks to @mbkumar pointing out the global.f90 was not linked.

I have made several non-trivial changes.

  1. Replace real to real(8) in the macros such that the python wrapper can normally convert real to double.

    I am trying to remove all the -r8 or -fdefault-real-8 option which seems redundent. But I found the reference HDF5 files for CI testing then need to be updated, as shown in https://github.com/PrincetonUniversity/SPEC/runs/1424404618?check_suite_focus=true.

    We should discuss this in future meetings. Right now, I just revert the commits and still use -fdefault-real-8.

  2. Add a simple driver at https://github.com/PrincetonUniversity/SPEC/blob/pywrapper/Utilities/python_wrapper/core.py.

Right now, the wrapper is not fully ready. You can access all the variables in the modules. But before making everything functioning, we should do the followings.

mbkumar commented 3 years ago

To add to Caoxiang's excellent work on the python wrapper, the same functionality can also be accomplished with cmake in the cmake branch. Instructions to compile using cmake are at the end of README.me file in the cmake branch.

One more thing we should do is integrate the python wrapper into the pythontools. This should be a straightforward exercise.

Bharat Medasani

Engineer Princeton Plasma Physics Lab (PPPL)

On Thu, Nov 19, 2020 at 10:03 AM Caoxiang Zhu notifications@github.com wrote:

The bugs are now resolved. Thanks to @mbkumar https://github.com/mbkumar pointing out the global.f90 was not linked.

I have made several non-trivial changes.

1.

Replace real to real(8) in the macros such that the python wrapper can normally convert real to double.

I am trying to remove all the -r8 or -fdefault-real-8 option which seems redundent. But I found the reference HDF5 files for CI testing then need to be updated, as shown in https://github.com/PrincetonUniversity/SPEC/runs/1424404618?check_suite_focus=true .

We should discuss this in future meetings. Right now, I just revert the commits and still use -fdefault-real-8. 2.

Add a simple driver at https://github.com/PrincetonUniversity/SPEC/blob/pywrapper/Utilities/python_wrapper/core.py .

Right now, the wrapper is not fully ready. You can access all the variables in the modules. But before making everything functioning, we should do the followings.

  • Interfacing sphdf.f90, which requires to figure out what hid_t and hsize_t are. Does @jonathanschilling https://github.com/jonathanschilling know the answer?
  • Move call getarg(1, ext) out of readin since it looks like there is no option to passing command-line arguments from the python side.
  • Write a mirror driver in python similar to xspech.f90. This probably requires some clean-ups to xspech.f90, like there should be no allocations in the main program.
  • Change MPI_COMM_WORLD to MPI_COMM_SPEC if we want to call SPEC with a sub-group of CPU rather than all the CPUs.
  • Update the GitHub actions to check the python wrapper automatically.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PrincetonUniversity/SPEC/issues/119#issuecomment-730433631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA62VEEAJ3DMWL6KHCRZWK3SQUXSPANCNFSM4TY2Q5TA .

zhucaoxiang commented 3 years ago

@jonathanschilling I am pushing the commits to pywrapper and you can pull & test it.

The things left are:

jonathanschilling commented 3 years ago

@zhucaoxiang Just a quick update: I got the pywrapper branch to compile on Debian 10 and I can execute the example. I will now start working on the open points listed above. Great work so far! :+1:

jonathanschilling commented 3 years ago

The pywrapper... branch has been merged into master and I therefore close this issue.