Skip to content
This repository has been archived by the owner on Jul 27, 2020. It is now read-only.

Fix Python lib build #372

Closed
keith923 opened this issue Oct 22, 2015 · 17 comments
Closed

Fix Python lib build #372

keith923 opened this issue Oct 22, 2015 · 17 comments

Comments

@keith923
Copy link
Contributor

Currently there is a CMake option, -DENABLE_PYTHON, that does not work. It errs in executing the CMakeLists.txt file in the tools subdirectory. But more than that, there is simply no CMake configuration for it in the tools/python directory.

@keith923 keith923 self-assigned this Oct 22, 2015
@keith923 keith923 added this to the APBS 1.4.2 release milestone Oct 30, 2015
@keith923
Copy link
Contributor Author

Part of what the ENABLE_PYTHON CMake flag does is configure this file. It looks like it's a way to run APBS indirectly using Python. It's not clear to me that this is useful.

It additionally set's the following variables in the resultant .py file:

set(SERVICEURL "http://kryptonite.nbcr.net/opal2/services/apbs_1.3")
set(PARALLELSERVICEURL "http://oolite.calit2.optiputer.net/opal2/services/apbs-parallel-1.3")

I'm not sure that the URLs are correct or not.

Do we need to generate "ApbsClient.py" at all anymore? Or can we rid ourselves of some useless cruft?

@kmonson
Copy link
Contributor

kmonson commented Nov 17, 2015

The script is a command line way to run the opal apbs service.

Those URLs are definitely wrong. Fixing this would require making those configurable.

Unless @sobolevnrm has a reason we should keep it around we can drop it. I've never had a user ask about it and I don't ever use it for testing. I'm not even sure if it works correctly.

@keith923
Copy link
Contributor Author

Thanks @kmonson for the info! I'm going to plan on dropping it unless @sobolevnrm pipes up to the contrary.

I have CMake building apbslib.py and _apbslib.so. My question now is: should I put them in the pdb2pqr directory someplace? Or do we just point the user at them and let them move them around?

@kmonson
Copy link
Contributor

kmonson commented Nov 17, 2015

I suppose we could copy them over to the pdb2pka directory. Sure, why not.

On Tue, Nov 17, 2015 at 12:36 PM, Keith T. Star [email protected]
wrote:

Thanks @kmonson https://github.com/kmonson for the info! I'm going to
plan on dropping it unless @sobolevnrm https://github.com/sobolevnrm
pipes up to the contrary.

I have CMake building apbslib.py and _apbslib.so. My question now is:
should I put them in the pdb2pqr directory someplace? Or do we just point
the user at them and let them move them around?


Reply to this email directly or view it on GitHub
#372 (comment)
.

@keith923
Copy link
Contributor Author

I'll see about doing just then! Thanks @kmonson.

@kmonson
Copy link
Contributor

kmonson commented Nov 17, 2015

You should also change the error message at the top of
psb2pqr/pdb2pka/apbs.py

On Tue, Nov 17, 2015 at 2:07 PM, Keith T. Star [email protected]
wrote:

I'll see about doing just then! Thanks @kmonson
https://github.com/kmonson.


Reply to this email directly or view it on GitHub
#372 (comment)
.

@sobolevnrm
Copy link
Member

Are you sure that those web services URLs don't work any longer? This was
a useful service and it would be good to keep it operational, if possible.

On Tue, Nov 17, 2015 at 12:36 PM Keith T. Star [email protected]
wrote:

Thanks @kmonson https://github.com/kmonson for the info! I'm going to
plan on dropping it unless @sobolevnrm https://github.com/sobolevnrm
pipes up to the contrary.

I have CMake building apbslib.py and _apbslib.so. My question now is:
should I put them in the pdb2pqr directory someplace? Or do we just point
the user at them and let them move them around?


Reply to this email directly or view it on GitHub
#372 (comment)
.

@keith923
Copy link
Contributor Author

The hosts in those URLs don't resolve any longer. The code that was generating the python file was pretty wonky -- I don't know if it's worked in years. We could create an issue to resurrect it all?

@sobolevnrm
Copy link
Member

That sounds like a good plan. Thanks!

On Wed, Nov 18, 2015 at 5:25 AM Keith T. Star [email protected]
wrote:

The hosts in those URLs don't resolve any longer. The code that was
generating the python file was pretty wonky -- I don't know if it's worked
in years. We could create an issue to resurrect it all?


Reply to this email directly or view it on GitHub
#372 (comment)
.

@keith923
Copy link
Contributor Author

Sure, no problem. What was that code doing? Just so that I have an idea what to put in the issue.

@sobolevnrm
Copy link
Member

It allowed users to run APBS on offsite servers from the command line --
everything was the same except for the command line options, so it could be
a drop-in replacement for APBS when used with visualization software like
VMD or PyMOL.

On Wed, Nov 18, 2015 at 6:12 AM Keith T. Star [email protected]
wrote:

Sure, no problem. What was that code doing? Just so that I have an idea
what to put in the issue.


Reply to this email directly or view it on GitHub
#372 (comment)
.

@keith923
Copy link
Contributor Author

Ah -- gotcha. Thanks @sobolevnrm!

keith923 added a commit that referenced this issue Nov 18, 2015
…b2pka is missing the APBS python library to instruct the user how to build said library.
@keith923
Copy link
Contributor Author

This works on OS X, and it will work on Linux (Ubuntu 14.04 LTS and RHEL 6.6), but requires that -DBUILD_SHARED_LIBS=ON be used as part of the CMake invocation because of this error:

[100%] Linking C shared module ../../lib/_apbslib.so
/usr/bin/ld: /pic/projects/apbs/apbs-pdb2pqr/apbs/externals/fetk/maloc/lib/libmaloc.a(vcom.c.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC

This puzzles me, because I'm fairly certain that I've built the _apbslib.so file on linux without having to resort to using shared libraries. Does this present such a problem that I should dig into it?

Also, @kmonson would you have time to try this out on your Windows box? You'll want to add -DENABLE_PYTHON=ON to your CMake invocation.

@kmonson
Copy link
Contributor

kmonson commented Nov 18, 2015

Yeah, I can give it a shot.

On Wed, Nov 18, 2015 at 8:00 AM, Keith T. Star [email protected]
wrote:

This works on OS X, and it will work on Linux (Ubuntu 14.04 LTS and RHEL
6.6), but requires that -DBUILD_SHARED_LIBS=ON be used as part of the
CMake invocation because of this error:

[100%] Linking C shared module ../../lib/_apbslib.so
/usr/bin/ld: /pic/projects/apbs/apbs-pdb2pqr/apbs/externals/fetk/maloc/lib/libmaloc.a(vcom.c.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC

This puzzles me, because I'm fairly certain that I've built the
_apbslib.so file on linux without having to resort to using shared
libraries. Does this present such a problem that I should dig into it?

Also, @kmonson https://github.com/kmonson would you have time to try
this out on your Windows box? You'll want to add -DENABLE_PYTHON=ON to
your CMake invocation.


Reply to this email directly or view it on GitHub
#372 (comment)
.

@keith923 keith923 assigned kmonson and keith923 and unassigned keith923 and kmonson Dec 1, 2015
@keith923
Copy link
Contributor Author

keith923 commented Dec 8, 2015

Kyle tried and got:

~/workspaces/github/apbs-pdb2pqr/apbs
$ cmake src
-- Building for: Visual Studio 9 2008
-- The C compiler identification is MSVC 15.0.30729.1
-- The CXX compiler identification is MSVC 15.0.30729.1
-- Check for working C compiler using: Visual Studio 9 2008
-- Check for working C compiler using: Visual Studio 9 2008 -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler using: Visual Studio 9 2008
-- Check for working CXX compiler using: Visual Studio 9 2008 -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Adding apbs_generic
-- With source files nosh.c;mgparm.c;femparm.c;pbeparm.c;bemparm.c;geoflowparm.c;apolparm.c;vacc.c;valist.c;vatom.c;vpbe.c;vcap.c;vclist.c;vstring.c;vparam.c;vgreen.c
-- With external header files nosh.h;mgparm.h;femparm.h;pbeparm.h;bemparm.h;geoflowparm.h;apolparm.h;vacc.h;valist.h;vatom.h;vpbe.h;vcap.h;vclist.h;vstring.h;vparam.h;vgreen.h;vmatrix.h;vhal.h;vunit.h
-- With internal header files
-- With library dependencies
CMake Error at CMakeLists.txt:38 (INSTALL):
install TARGETS given no ARCHIVE DESTINATION for static library target
"apbs_generic".
Call Stack (most recent call first):
generic/CMakeLists.txt:44 (add_sublibrary)

@keith923
Copy link
Contributor Author

This commit fixed some problems with the naming of the python library: bbcb70b

Apparently .dylib does not work on OS X, and the CMake SWIG module doesn't use the OS specific extension anyway.

I'll test this commit on Linux and OS X and then close the issue.

keith923 added a commit that referenced this issue Dec 16, 2015
…enable shared libraries to build the python lib on unix. Closes issue #372.
@keith923
Copy link
Contributor Author

Looks good on Win32, Linux (RHEL) and OS X.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants