loftytopping / PyBox

A box-model that automatically creates and solves equations used to describe the evolution in atmospheric composition using Python with Numba and, optionally, Fortran.
GNU General Public License v3.0
37 stars 11 forks source link

JOSS Review: Docker questions #7

Closed lheagy closed 6 years ago

lheagy commented 6 years ago

Originally from @highendcompute in openjournals/joss-reviews#755

I pulled updates as of 3rd Aug, stopped and removed all previous pybox containers, removed all previous pybox, & ubuntu images (presume was intermediate later). From this point:

for the Dockerfile I had to add an extra "no" to prevent hanging on final question:

Do you wish the installer to prepend the Anaconda3 install location
to PATH in your /root/.bashrc ? [yes|no]
[no] >>> 
Appending source /root/anaconda3/bin/activate to /root/.bashrc
A backup will be made to: /root/.bashrc-anaconda3.bak

For this change to become active, you have to open a new terminal.

Thank you for installing Anaconda3!

===========================================================================

Anaconda is partnered with Microsoft! Microsoft VSCode is a streamlined
code editor with support for development operations like debugging, task
running and version control.

To install Visual Studio Code, you will need:
  - Administrator Privileges
  - Internet connectivity

Visual Studio Code License: https://code.visualstudio.com/license

Do you wish to proceed with the installation of Microsoft VSCode? [yes|no]
>>> 

my general comments in terms of optimising a user's build via docker: 1) is it possible to use miniconda then build up only what is required? (base ubuntu vmem 83 MB to final layer being 4.4 GB) 2) It is recommended for Docker containers to NOT run as root, but as a user (set up in Dockerfile)

please note that I am happy with this current version meets the JOSS requirements, the above are merely suggestions/questions for potential improvements in future versions

I will open other comments re running the Docker image and on building outside of Docker

lheagy commented 6 years ago

continued...

running from docker image, i appear to hit a problem:

[20:48:10] ~/DT_pybox/PyBox$ docker run -it pybox root@01a532291b93:/Code# whoami root root@01a532291b93:/Code# cd /Code/Git_repos/PyBox/ root@01a532291b93:/Code/Git_repos/PyBox# python Gas_simulation.py Opening file MCM_APINENE.eqn.txt for parsing Calculating total number of equations = 836 Parsing each equation Total number of species = 305 Saving all equation information to dictionaries Saving the mechanism dictionary as a pickled object for later retrieval Converting rate coefficient operations into Python-Numba file Creating Python-Numba functions and sparse matrices for product multiplications and dydt function Creating Python-Numba Jacobian Traceback (most recent call last): File "Gas_simulation.py", line 163, in <module> Parse_eqn_file.write_gas_jacobian_numba(filename,equations,num_species,loss_dict,gain_dict,species_dict2array,rate_dict_reactants) File "/Code/Git_repos/PyBox/Parse_eqn_file.py", line 2987, in write_gas_jacobian_numba f.write('#\xa7# \n') UnicodeEncodeError: 'ascii' codec can't encode character '\xa7' in position 1: ordinal not in range(128) root@01a532291b93:/Code/Git_repos/PyBox#

lheagy commented 6 years ago

building on linux server (uname: Linux mkb-debian-vm 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2+deb8u3 (2017-08-15) x86_64 GNU/Linux)

general comments (do not need fixing but please consider): ) maybe state if python2 is okay or whether it has to be python3 (I'm testing with python2) ) why both numpy and scipy? isn't it possible to use just one or the other

In terms of the dependencies for Python (not my primarily language I admit), Debian does not have any form of conda in its package manager suite nor do I see URL/.deb for alternative. It would be useful to clarify whether any Python package manager (eg pip) will suffice or whether there is a good reason to use anaconda.

In order to validate your assertions I have used a VM and run the anaconda install script. The following is based on this:

In terms of testing this build, I again get a (de)coding error:

[14:34:53] ~/DT_pybox/PyBox$ python Gas_simulation.py File "Gas_simulation.py", line 38 SyntaxError: Non-ASCII character '\xe2' in file Gas_simulation.py on line 38, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details

lheagy commented 6 years ago

@loftytopping, @highendcompute, from your conversation these issues look to be resolved? and superseded by #6. Please feel free to close this @loftytopping if that is the case

loftytopping commented 6 years ago

Yes thats right @lheagy many thanks. Ive now created a new enhancement issue for future development, to provide a 'lightweight' docker image.