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

WIP - installation to non-system directory - use anaconda #26

Closed
wants to merge 1 commit into from

Conversation

bloyl
Copy link

@bloyl bloyl commented Dec 20, 2018

This

  1. uses anaconda to standardize python installation.
  2. installs hnn, iv and nrn to a non-system directory, so doesn't need sudo.
  3. uses environment variable HNN_ROOT to identify where hnn's root src directory is.

closes issue #25 (@jasmainak)

@bloyl
Copy link
Author

bloyl commented Dec 20, 2018

questions

  1. can hnn.sh be removed? if not does it really need to modify the path or can we assume the environment is already setup?

  2. I'm currently testing (on centos and ubuntu box) by starting the GUI and clicking the run simulation button. Is there a more extensive set of tests i can/should do?

@samnemo
Copy link
Member

samnemo commented Dec 20, 2018

A. Windows/Mac instructions currently tell users to start HNN via python hnn.py hnn.cfg
B. hnn.sh is still needed when invoking HNN from Noam Peled's MMVT: https://github.com/pelednoam/mmvt
C. For Linux we've been telling people to call the hnn file (which just has cd to /usr/local/hnn then python hnn.py hnn.cfg), so hnn and hnn.sh could be updated slightly to use your new HNN_ROOT env var
D. For testing, we usually tell people to run the simulation and make sure the output roughly corresponds to that shown in hnn.brown.edu tutorials; in the future we plan to have the installer run a simulation and check the output is correct (e.g. via hash of output data); a further test is after running the simulation to make sure the different view menu items display output

@bloyl
Copy link
Author

bloyl commented Dec 20, 2018

ok I'll leave hnn.sh alone for now. although in the future it would make sense to consolidate hnn and hnn.sh since they seem to do very similar things.

@samnemo
Copy link
Member

samnemo commented Dec 20, 2018

OK, I'll merge the PR once you confirm that tests have passed. Thanks

@samnemo
Copy link
Member

samnemo commented Dec 20, 2018

Robert is checking out the changes first...

@bloyl
Copy link
Author

bloyl commented Dec 20, 2018

You should certainly hold off until i change the title of the PR to something like [MRG] as opposed to [WIP].

everyone should also review the changes for other/unforeseen issues, for instance the use of HNN_ROOT in hnn will break existing installations when they are updated since the /etc/profile.d/hnn.sh won't have it setup. I'm not sure how big a deal that is for your existing users but it should be something you consider.

# create conda environment
# TODO: Replace with enviroment.yml file?
conda create --name hnn python=3 cython scipy numpy matplotlib pyqt pyqtgraph pyopengl libtool=2.4.2
source activate hnn
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's a good idea to force a virtual environment. You can give an environment.yml file so users are free to create one if they want. But forcing the virtual environment means they have different versions of the same package on their system.

So for instance, I could have a numpy=1.13 in the mne virtual environment and numpy=1.19 in the hnn virtual environment. Now, if I do some computations, I could get different results simply because I'm in different virtual environments.

echo "Installing hnn, iv and nrn into $install_prefix"

# check system dependencies.
# TODO: What else should be checked for
Copy link
Collaborator

Choose a reason for hiding this comment

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

it worked for me without installing any system dependencies on CentOS

Copy link
Collaborator

Choose a reason for hiding this comment

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

Try again with a fresh base install. There are many dependencies not included by default.


# build, configure and install nrn
cd ../nrn
#git checkout be2997e
Copy link
Collaborator

Choose a reason for hiding this comment

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

to be removed

ln -fs ${install_prefix}/hnn/hnn ${install_prefix}/${CPU}/bin/hnn

# useful environment variables
env_setup_fname=${install_prefix}/hnn_profile.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is this file? I couldn't find it in the repository

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's created in the following lines.

echo 'source activate hnn' | tee -a ${env_setup_fname}
echo 'export PATH=$PATH:${install_prefix}/$CPU/bin' | tee -a ${env_setup_fname}
echo 'export HNN_ROOT=${install_prefix}/hnn' | tee -a ${env_setup_fname}
echo 'export PYTHONPATH=${HNN_ROOT}' | tee -a ${env_setup_fname}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I skipped all these steps and just added install_prefix/CPU/bin/ to my path.

@bloyl
Copy link
Author

bloyl commented Dec 20, 2018

Luke,

Thanks for starting an install script for CentOS with Anaconda.

I applaud the idea behind installer/centos/anaconda_installer.sh in that (1) it doesn’t require root permissions, and (2) it takes advantage of anaconda if available.

Unfortunately, it’s not ready to be merged, but I hope the below comments will help.

The main issue is that while it doesn’t use sudo, the only reason it can do so is that it assumes people already have most of the things that the other scripts use root to install. In particular, this won’t work for off-the-shelf CentOS with anaconda.

An install script should (1) work for people who have the stated prereqs (here CentOS and anaconda), (2) not require users to make choices that could break the install, and (3) not break any of the other install scripts. This pull request is not there yet on any of these criteria.

As proposed, the pull request has a trivial (but fatal) error for CentOS users who don’t have anaconda installed:

· The new line 76 in both centos6-installer.sh and centos7-installer.sh will fail with a permission denied error (they need a “sudo” on those tees), which with the modification to the hnn script means that the “cd ${HNN_ROOT}” will fail and thus hnn won’t run at all.

The anaconda_installer.sh script requires people to already have a development environment:

· Obviously, it assumes the existence of anaconda, which I’m mostly okay with, but it ought to check. (If not present, it ought to fail immediately, lest the failure message get buried.)

· It requires gcc to be available. (There is no check per se, although not having gcc will cause the mpicc check to fail, although installation attempts to continue anyways).

· It requires mpicc and mpiexec to be available and on the path. It does at least check for this (yay) but it doesn’t exit immediately if not available (boo).

o Note that by default even if you do a yum install of openmpi, that doesn’t actually place mpicc on the path, so this isn’t something we can assume

· It requires git to be available. There is no check. (Arguably none of these things actually need git and we can make do with downloading zipped sources.)

· It requires autotools to be installed. (There is no check for this, but without this, NEURON won’t build)

· It requires ncurses-devel (or equivalent for termcap or curses) to be installed. (There is no check for this, but without this, NEURON won’t configure)

· It requires libXext-devel. Without this, Inteviews won’t compile, but the error will be buried. Now strictly speaking, HNN doesn’t need NEURON to have Interviews, but that is required with the configure options passed to NEURON in this script.

The anaconda_installer.sh script requires users to interactively make correct choices on a clean system:

· When creating the “hnn” environment, it prompts for confirmation about installing 55 packages (all together, thankfully). If the use chooses “no”, the script won’t detect it, but hnn won’t run.

Typing “hnn” still won’t just work even when all of the above have been addressed:

· It requires the “hnn_profile.sh” script to be run before the command can be invoked

o Without this, hnn isn’t necessarily on the path and the hnn conda environment is not necessarily active

o Note that this script is not executable, so it would need to be run with “source” or chmod’d first

· Once I did that, I still ended up with something where I couldn’t run a simulation, because it never compiled the mod files. This may or may not still be true when

Finer points that aren’t necessarily deal breakers, but aren’t great:

· If no install prefix is specified on the command line, it will display the “please specify an install prefix” and continue (and not exit). I’m not 100% sure, but I believe even if all of the above are fixed, this will still fail to create a working hnn install, because it will attempt to install on whatever folder it was run from, which means that line 75 will delete the hnn folder and that’s pretty much that. This could be avoided by making lines 75 and 77 only run when the install_prefix is (1) non-empty and (2) not equal to the $PWD.

· The script doesn’t check to see if the hnn conda environment already exists.

· The hnn script now does a “cd” to ${HNN_ROOT}, which means that paths for files saved and loaded will start out in ${HNN_ROOT} instead of whatever directory hnn was launched from.

Recommendations:
· Try again, testing on a clean CentOS virtual machine with a fresh anaconda install.
· Try to keep it from requiring root permissions, but getting the prerequisites to work (see the list under “already have a development environment”) will be non-trivial.

I’m happy to discuss any of these points or to give feedback on a revised pull request.

Happy holidays!

Best,

Robert McDougal

@bloyl bloyl closed this Dec 20, 2018
@bloyl
Copy link
Author

bloyl commented Dec 20, 2018

opps didn't mean to close the PR

@bloyl bloyl reopened this Dec 20, 2018
@jasmainak
Copy link
Collaborator

@bloyl my personal feeling is that Robert and me are coming from two different communities, so we have different way of doing things :)

@samnemo what is your vision for the package? I see that the code is mostly in python. Python packages usually assume anaconda and then use a setup.py with a requirements.txt or an environment.yml. The installation of other non-python dependencies nrn and iv could be clearly documented (with and without sudo) in a README file but I think it's overkill to require sudo for this.

@jasmainak
Copy link
Collaborator

Try again, testing on a clean CentOS virtual machine with a fresh anaconda install.

FWIW, I ran all these steps on a new CentOS distribution I got at the lab -- without installing anything new. autotools, git etc was already available.

@samnemo
Copy link
Member

samnemo commented Dec 20, 2018

@bloyl - Let's try to take @ramcdougal suggestions into account before finalizing any changes.
@jasmainak - yeah, most of the new code is in Python but without NEURON, nothing would work, so that's why we didn't go the pure Anaconda route. If you have a good or alternative installer, we could test it internally, as well as with the other users, and see what people prefer.
Thanks!

@ramcdougal
Copy link
Collaborator

@jasmainak It's not a base install if it has development software available. Download and install CentOS on a virtual machine for yourself without selecting any extra installation packages (gnome is fine). You won't have any of those dependencies. If it's not available in the base CentOS, you cannot assume that other people will have it.

@jasmainak For an Anaconda specific install, we can certainly assume Anaconda, but in my experience use of non-system Pythons is rare on Linux. Even on other platforms, there are many competitors to Anaconda.

Note that there's no longer any reason to build NEURON from source. The released version should run just fine. @bloyl found a problem the other day when there was a change in the development version that broke HNN. We worked around it and HNN is better now, but it was a bug in a recent-ish change to NEURON that broke things. Using the released version would have avoided this issue.

@jasmainak
Copy link
Collaborator

I totally agree that we should use stable version of NEURON if possible! That would be great. Doesn't NEURON ship binaries that could be readily used? If this is the case, we can do away with all the sudo installations, right?

in my experience use of non-system Pythons is rare on Linux

this I would disagree with :) anyway, it's not too much to ask people to download miniconda and then install the packages. It's certainly easier than asking IT people to install packages for me every time there is an update.

@jasmainak
Copy link
Collaborator

also, if you don't like going the Anaconda way, we can go with requirements.txt so that everything is installed with pip. I believe this should be fine if Neuron ships the binaries

@bloyl
Copy link
Author

bloyl commented Dec 21, 2018

@ramcdougal Thanks for the feedback. I guess the intention of my script wasn't clearly defined. The idea was to provide a means to compile the important things needed by and supplied by hnn. You are correct the system level installs need sudo to install and thus need to be done by a sys-admin etc. building nrn, iv, and hnn (especially development versions) shouldn't need those permissions and shouldn't require installing to system directories as that can easily effect other installed software (particularly true of system python changes). That however is up to the managers/maintainers of the project which is fine.
My 2 cnts would be to go with a distutils setup.py approach so that hnn could one day by installed via pip. but that is just me.

I'm going to close this PR as i don;t feel like its particularly useful at this time. I put the main scripts from the PR in https://gist.github.com/bloyl/89fa7cb051aff385298f9fdd1533a8ac and i'll probably keep my branch open until i forget and delete it :). if anyone wants to use them for something

My immediate goal is to get hnn working on our production workstations and our hpc cluster. In order to do that I need to have hnn installed somewhere other than /usr/local, which is now required by the hnn script.

hnn/hnn

Lines 1 to 2 in de20760

cd /usr/local/hnn
python3 /usr/local/hnn/hnn.py /usr/local/hnn/hnn.cfg

I'm going to open a separate PR to fix that. and then start to use the program and see what other hic-ups i run into.

@bloyl bloyl closed this Dec 21, 2018
@samnemo
Copy link
Member

samnemo commented Dec 21, 2018

If you want to add additional installation scripts that allow custom installation of HNN, or allow specification of install location of HNN using current scripts, that's OK. We could more easily merge that and allow people to test it if it doesn't impact the default function of current installers. Previous versions allowed installing HNN to directories other than /usr/local . Thanks

@jasmainak
Copy link
Collaborator

I agree with @bloyl that installation via setup.py is what I know as kosher in the python world.

@samnemo would you have some time to walk me through the scripts at some point in January? I'll try to go over the HNN tutorials / scripts in the meanwhile

@ramcdougal are you aware of any pure python implementations of Neuron? I think it would be better for me to start with barebone implementations using only numpy and scipy and/or jupyter notebooks to get a more solid foundation. I found this -- it looks quite promising but maybe you know of others?

@samnemo
Copy link
Member

samnemo commented Dec 21, 2018

@jasmainak - sure, we can setup day/time to go over the code in Jan. Also, that toolbox you mentioned is just some Python-based add-ons for NEURON.

@jasmainak
Copy link
Collaborator

awesome! let's may be discuss this over email :)

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.

None yet

4 participants