PySCeS / pysces

The official PySCeS project source code repository.
https://pysces.github.io
Other
34 stars 10 forks source link

PySCeS behaviour suggestions for discussion #23

Closed jmrohwer closed 7 years ago

jmrohwer commented 7 years ago

Recently, through various use cases, a number of features/behaviours of PySCeS have started nagging me. The use cases range from PySCeS Toolbox applications to using a PySCeS model in optimization routines to fit parameters in kinetic models to experimental datasets. The main issues are:

  1. PySCeS changes the working directory (to $HOME/Pysces or C:\Pysces depending on your OS) every time you import the module or load a model.
  2. There is no way to change the model/output directory to the current directory by default.
  3. The first call to mod.doLoad() does not actually (re)load the model.
  4. There is still significant output during simulation and scanning, which is not silenced completely by mod.SetQuiet(). Also importing the module and loading a model produces output which one may want to silence under certain circumstances.

Before diving into code changes, I'd like to hear your opinions. My reasoning is as follows:

Working directory (points 1 and 2)

I am moving more and more away from having all PySCeS models in a single directory to having all related files for a particular project (i.e. scripts, PSC files, data in spreadsheets, Jupyter notebooks, etc.) in the same directory. Almost all my notebooks start along the lines of:

import os
bdir = os.getcwd()
import pysces
mod = pysces.model('mymodel.psc',bdir)
os.chdir(bdir)

I agree that the default behaviour should be maintained for backwards compatibility but it would really help me a lot to implement a way to set the pysces.PyscesModel.MODEL_DIR and pysces.PyscesModel.OUTPUT_DIR to point to the current working directory through a config setting (I realize you can change it in the .ini file but it still points to a fixed path, I want it to point to the CWD). As well as to disable the forced directory change after importing pysces or loading a model.

mod.doLoad()

Just by chance, by reading the docstring of that method, I realized that this method has been replaced by mod.ModelLoad(). Even I as regular user and part contributor wasn't aware of this :-) So maybe add some output advertising the new method to when the user calls mod.doLoad() for the first time?

Standard output

This has led Carl to even write a separate method/decorator (silence_print()) for psctb which kills all standard output. It should be possible to activate a setting that really removes all standard output. A more fine-grained distinction between stdout and stderr may also be useful, so that true error messages are still displayed. (Current code makes extensive use of print statements).

Ideas/comments?

bgoli commented 7 years ago

Some very interesting and good points, some easier to deal with than others. I'll address them one-by-one, first an easy one:

doLoad()

Originally to load a model you needed to instantiate a PysMod model object and then call doLoad() to generate all the internal data structures. Around 0.9.1 I received quite a few complaints from users that this was irritating. I then moved the model initialization into the ModelLoad method which is now called automatically from the model constructor.

This led to a backwards compatibility problem as now if scripts called the doLoad() method they would initialise the model twice. This led to the current configuration where the first call to doLoad() is ignored, etc. So ModelLoad was never a replacement ... doLoad was supposed to disappear!

However, I see there is utility in having a reload type function which re-initializes the model to its default parameters, which is how you would like to use it.

My proposal would be to create a reload method that calls ModelLoad and finally deprecate doLoad altogether (it can simply print a message to use reload).

jmrohwer commented 7 years ago

I see no need for a separate reload method as I can call ModelLoad() directly. I just wasn't aware of it :-) I use this feature quite regularly to re-initialize a model to its default params.

jmrohwer commented 7 years ago

I have hacked around the issue of the working directory tonight and believe I have fixed it. You can just specify in your .pys_usercfg.ini:

output_dir = ./
model_dir = ./

Not sure what the syntax would be under Windows, haven't checked yet. In addition to this I had to change some code how the InfixParser is called but now it works. Will create a branch and submit a pull request tomorrow.

I have also cleaned up the code so that silentstart now really makes a silent start. The other issues with standard output are still outstanding. TBC...

bgoli commented 7 years ago

mod.SetQuiet

SetQuite only really sets the solver algorithms verbosity and as you note doesn't really have any effect on PySCeS user feedback messaging. One needs both behaviours, if new users see nothing happening they assume the software has crashed and the opposite goes for when you are using PySCeS as an API.

There is a nice solution, which would take quite a bit of work but actually kill two birds with one stone and that is to create a PySCeS print method that wraps the Python 3 style print() function then replace all the Python 2 print 'blah' type statements with the new one.

This would have two advantages:

As far as the startup message that is displayed when PySCeS is imported. This can already be turned off by setting the following option in .pys_usercfg.ini

[Pysces] silentstart = True

bgoli commented 7 years ago

I don't know how to reply to a previous message but reLoad just sounds better. I've just updated the year in the copyright notice and some other stuff and pushed them so branch away :-)

jmrohwer commented 7 years ago

As discussed above, I have implemented a mod.reLoad() method in commit 6b30f90a3c87e934fd6d7e4dada23b96a6e74154 which basically just calls mod.ModelLoad(). I have not created a new branch for this but just committed to master :-)

I am now closing this issue, as the rest will be taken care of by the move towards Python 3 compatibility and the new PySCeS print method, see #26 .