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

Add new feature to fluid2d: EMS #3

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Add new feature to fluid2d: EMS #3

wants to merge 36 commits into from

Conversation

kegelkugel
Copy link
Collaborator

@kegelkugel kegelkugel commented May 17, 2019

EMS stands for Experiment Management System and is a powerful, new way to handle big sets of experiments. It is an optional feature that needs to be activated by the user if wants to profit from it. An example of its usage is shown in the new experiment "Waves with EMS".

EMS stands for Experiment Management System and is a new way to handle
big sets of experiments.  It is an optional feature that does not
influence the way fluid2d works unless activated by the user.  An
example of its usage is shown in the new experiment `Waves with EMS`.
@kegelkugel kegelkugel requested a review from pvthinker May 17, 2019 15:20
@kegelkugel
Copy link
Collaborator Author

kegelkugel commented May 17, 2019

In the future, a user interface to access the database could be developed. Until then, the database can be read with sqlite3 or a browser add-on like SQLite Reader: https://addons.mozilla.org/en-GB/firefox/addon/sql-reader/

UPDATE: the newest commit contains a simple user interface for the command line which offers already some additional features compared to a simple SQL reader.

UPDATE: the user interface is now quite advanced and offers a lot of additional features for data analysis and data handling.

@kegelkugel
Copy link
Collaborator Author

The failing check complains about a possible SQL injection attack. This was solved with the second commit. The difficulty is that sqlite3 does not allow using placeholders for column names or table names, as explained here: https://stackoverflow.com/questions/31277027/using-placeholder-in-sqlite3-statements
The code uses the safe placeholder wherever possible. In the cases where this is technically not possible with the Python module, additional checks were added at the time the experiment file is parsed. They prohibit SQL injection attacks, as far as I can see.

kegelkugel added 14 commits May 20, 2019 19:25
The development of this tool is not yet finished, but already works and
provides some useful functionality:
 - looking at the database of experiments
 - opening his/diag/mp4 files for specific experiments
Shell of the Experiment-Manager has a new command "verbose" to
configure whether one wants to see the messages printed by external
programmes or not.
Also the way to enable and disable columns in the table view was greatly
improved.
New internal behaviour is implemented that allows to hide all the
columns of the experiment table in the EMS Interface.
With this, the help-text of the functions for this was updated.
Further changes:
Add the switch -v to some commands to display all information in the
database, even hidden parameters.
Improve help messages for these commands.
The last column in the table view of the EMShell, which displays the
comment of an experiment, can now automatically adopt its width to the
width available in the terminal window.  Also the full comment man be
displayed every time instead.

The option to start the EMShell without fluid2d activated is removed.
It did not fit well into the general behaviour of the fluid2d-system.

New function is added to the EMShell to display free disk space.
A new setting is added to the EMShell which, when activated, prints the
value of the datetime column in a format which is easily readable by
humans, for example by saying "15 min ago" instead of the exact date and
time.
The ems was slightly modified to prevent unexpected behaviour in the
rare case that datetime.now() returns a time with 0 microseconds.
Fix some problems in the last commit, so that the code is more flexible
and a lot shorter now.
@kegelkugel
Copy link
Collaborator Author

The result of the Codacy checks are a bit annoying, but I don't think they are problems.

The one issue found many times by Codacy is, as mentioned before, the possibility of SQL injection. However, the Python module used does not allow a better way to format the SQL commands and already prevents the usage of typical SQL injections. Furthermore, it's just the user harming themself.

The other issue appearing several times is “method could be a function”. However, in the cases found by Codacy, making the method a function would be against the logic of the Cmd class used, since the do_, help_ and complete_* functions must be methods of the EMShell-class.

Codacy also complains about the usage of subprocess, but that's also just another way for the user to harm themself, so not an issue that needs to be fixed, since subprocess does a good job.

And finally, the unused variables in the for-loops make it in fact easier to understand the code, so I would not change those.

This commit also contains changes in same names, docstrings and
comments.
With this commit it is possible to remove tables from the database and
to change to comment of entries in the database.  The behaviour and the
help texts of several commands are made uniform.  Instead of an ID, it
is now possible to say "last" or "-1".

A bug in the ems is fixed which caused a crash when a table was empty.
When fluid2d runs on multiple cores, the name of the his file displayed
at the end was wrong.  The filename that was printed after the
simulation finished was the name of one of the his-files which was
joined into one big his-file and then removed.  With this commit,
fluid2d prints the correct name of the his-file after the run even when
using multiple cores.  The same is true for the flux file.

Creating an mp4-file while fluid2d runs on multiple cores was possible
so far, however, it resulted in the creation of just one file which
shows only some part of the domain, not fully.  Therefore it was only
limited usefully.  Furthermore it led to a crash, since several
fluid2d-processes called "finalize" on the same file.  Also creating an
mp4-file, which is not really helpful wastes resouces.  Therefore, this
commit disables the possbility to generate mp4s when fluid2d runs on
multiple cores.  A warning is printed instead.

The member variables of Output hisfile_joined and flxfile_joined were
unused and are now removed.
Also improve the handling of verbose and silent mode in EMShell.
Usage of the parameter timespec in datetime.isoformat() made it
impossible to use EMS with Python 3.5.  The parameter timespec is
removed now, which required a bit of fine-tuning in th EMShell.
The EMShell itself still requires Python 3.6 however.

Also improve one error message in the EMS.
It proved useful to start the EMShell by specifying the path to the
experiment folder instead of always taking the default expdir path.
This possibility, which existed before, is re-added.
It is proposed by Codacy to not use an empty list as the default value
of a function: http://pylint-messages.wikidot.com/messages:w0102
So instead, the value None is used.
@kegelkugel kegelkugel self-assigned this Jun 18, 2019
@kegelkugel kegelkugel added the work in progress Do not merge a pull request with this label into master. label Jun 18, 2019
The EMS is almost fully reworked in this commit.  It now contains the
long desired feature of specifying multiple values in the experiment
file and automatically running one experiment for every possible
combination of these values.  Once again, this is demonstrated in the
Waves experiment.  Furthermore, the EMS supports now natively
multi-threading, which required some manual hacks before.  However, this
required to add the EMS to the Param class, which, at the end, made the
whole implementation a lot cleaner and easier for the user.
In consequence, this commit modified, for the first time, files of the
standard distribution of fluid2d.  The influence was kept as tiny as
possbile.  As before, the EMS does not make a difference for the user
unless he activates it, which brings now a whole new experience!
@kegelkugel kegelkugel removed the work in progress Do not merge a pull request with this label into master. label Jun 18, 2019
kegelkugel and others added 7 commits June 19, 2019 11:57
The Experiment-Manager EMShell together with its extensions is moved
into its own directory to have a clearer arrangement and structure.
This commit brings a lot of code improvements, most of them in the
DB-connection class, which was simplified a lot.

Furthermore, new functionality was implemented to add or remove columns
from a table.

The requirement to have numpy installed was removed from the
EMShell (apart from the EMShellExtensions, which are considered
optional) and matplotlib was made an optional requirement.  A lot (but
not all of the functionality of EMS) can be used without matplotlib, so
it is now possible to run EMShell on a computer without matplotlib.

When matplotlib is used, a different backend is now chosen by default,
which allows the user to modify the plot manually.  This is a big
advantage when the user wants to create nice figures.

Also a new colormap was introduced that can be chosen by the user.

Finally, comments were added and improved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants