-
Couldn't load subscription status.
- Fork 4
Fix installation #11
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
base: main
Are you sure you want to change the base?
Fix installation #11
Conversation
|
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR 7 also introduced a MANFEST.in file and changed the setup.py file to override the build command, enabling the compilation and inclusion of the shared object from cFMS from a single call to pip install. Could this PR be changed such that subprocesses called during the override of build will check for the existence of the shared object file, whether it is compiled from a totally separate method, or from the compile.py file, otherwise it will use the compile.py file to generate it? This will enable the package to still be completely installed via a single pip install.
|
@mlee03 - I agree with @fmalatino that we need the most flexible installation processes possible. For Pace work, we want to ensure a user doesn't need a manual multi-stage installation process that may not be well understood. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With your suggested change to provide pre-compiled versions of FMS and cFMS by default, you will need to remove the submoduled in cFMS.
| # path to the installed yaml library | ||
| yaml = "/opt/libyaml/0.2.5/GNU/14.2.0/" | ||
|
|
||
| # path to the installed netcdf library | ||
| netcdf = "/opt/netcdf/4.9.3/GNU/14.2.0/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not ideal as they are hard-coded for a specific versions and a specific machine. I know this is addressed in the Readme.
| FC = "mpif90" | ||
| CC = "mpicc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires an MPI library be accessible. You'll need to mention that as well. Is there any chance this could become an issue if mpi4py and the MPI used here are different flavors?
|
I'll add back in the compile and installation of FMS and cFMS during pip install. However, the current method is not reliable. Reading up a bit hints that we should go with a meson build system for cFMS. Building cFMS is much simpler than FMS because there is no inter-module dependencies in cFMS, so I don't think it will be that difficult to set up the build without using autotools and cmake. And packages like numpy and scipy that have compiled backend libraries use meson. I'll change this PR into a draft. |
Description
With the current setup,
pip installwill not distribute the compiled cFMS library when installing pyFMS. The following changes have been made to fix the installation issue:FMS and cFMS compilation is no longer part of pyFMS pip installation. Instead, the users will need to compile
the libraries manually by using the
compile.pyscript. This way, pip installing pyFMS will be less time consuming and compiling cFMS/FMS will be more foolproof. The libraries will be installed topyfms/libAdds a
MANIFEST.into include the compiled libraries as part of pip install.Adds a README for installation directions