General style updates

[LiamPattinson]

There are a few style changes that could be made to make the project more 'pythonic' and to make future development easier. The aim here wouldn't be to change any behaviour, but instead to simplify some code and reduce the potential for bugs to creep in. Let me know your thoughts on any of these suggestions.

while loops

A common pattern throughout the project is to use while loops in places where a for loop might be safer:

# while version
i = 0 # developers might forget to set this when 'i' is reused within a function
while (i < num):
    # do something...
    i += 1 # developers might also forget this line, leading to infinite loops

# for version
for i in range(num):
    # do something...

There are also places where loops are used to build lists, and they could instead be replaced with a list comprehension:

# while version
my_list = []
i = 0
while (i < list_len):
    my_list.append(some_function(i))
    i += 1

# list comprehension version
my_list = [some_function(i) for i in range(list_len)]

This might perform a little better (although it's never guaranteed), and it leads to much more compact code. I find them quite easy to read, but I understand that some other programmers aren't too keen on them, so it's personal preference if they're used or not. It's easy to fall into the trap of over-engineering list comprehensions until they're near-unreadable, so it's best to keep them simple.

Repeat code

There are some places where a long line of complex arithmetic is repeated on several lines. It'd be better to break these lines down into a few variables, as then there's less risk of developers modifying a line of code but forgetting to make similar changes to later lines (or simply making a mistake when updating later lines). e.g:

https://github.com/AleksandarJ1984/FEDM/blob/720e18015a994defab8c44d01b433bc4ddefaf5d/fedm_modules/functions.py#L220-L228

Could modify to something like:

# Ideally, should break this down further to aid readability
common_part = 2.0*pi*exp(u)*((((1.0+2.0*dt/dt_old)/(1.0+dt/dt_old))*(u - (pow(1.0+dt/dt_old, 2.0)/(1.0+2.0*dt/dt_old))*u_old\
         + (pow(dt/dt_old, 2.0)/(1.0+2.0*dt/dt_old))*u_old1))*v/dt)*r*dx
if (equation_type == 'reaction'):
    return common_part - 2.0*pi*f*v*r*dx
elif (equation_type == 'diffusion-reaction'):
    return common_part - 2.0*pi*dot(-grad(D*exp(u)), grad(v))*r*dx - 2.0*pi*f*v*r*dx
elif (equation_type == 'drift-diffusion-reaction'):
    return common_part - 2.0*pi*dot(Gamma, grad(v))*r*dx - 2.0*pi*f*v*r*dx
else: # handle incorrect 'equation_type'
    raise ValueError(f"Equation type {equation_type} not recognised")

Formatting with an autoformatter/linter

I've found some of the longer lines in the project difficult to read, especially if I'm looking at two files side-by-side in my terminal. We could use a tool like Black to automate the formatting of the project, which in my experience tends to do a really good job of producing consistent and readable code (it sometimes does questionable things, but if it makes your code less readable it's usually an indication that you're doing too much on one line anyway).

We could also use flake8 to enforce style further and catch any potential bugs (it's good for catching unused variables). These can both be configured using setup.cfg/pyproject.toml, which would be implemented for Issue https://github.com/AleksandarJ1984/FEDM/issues/1.

[LiamPattinson]

https://github.com/AleksandarJ1984/FEDM/blob/720e18015a994defab8c44d01b433bc4ddefaf5d/fedm_modules/file_io.py#L16-L17

Rather than opening a file at the start of the run and passing around a file handle, it's better to just pass around a path to the file. You can open and write to a file without deleting the contents using with open("myfile.txt", "a") as myfile:. This way, you avoid bugs where the user may try to write to a file that has already been closed, or the user forgets they have to close the file manually when they're done with it.

[markus-m-becker]

(Partly) solved by @LiamPattinson, close issue?

[RaphaelPile]

Several comments:

• The first time step of the solution cannot be saved in the examples.
• If the dt_init is ill-chosen, the time loop never stops in time-of-flight.
• A lot of code is copy-pasted between the different example, and it is even completely copy-pasted between example and tests. However, it is really helpful to have everything at the same place when you are learning how everything is working. I propose to create a Jupiter Notebook tutorial, which could be in the format of the current examples, and then refactoring code in the examples/tests.

[AleksandarJ1984]

@RaphaelPile Thank you for reporting the issues. I will check them out.

As for the copy-pasting of the parts of the code between the examples, the idea was to illustrate the code going from simple example to more complex ones but, at the same time, to have some continuity between them. Each builds on the previous one, so I will probably keep this structure, at least for now.

Regarding the Jupyter notebook tutorial, since many people use it, it would be quite helpful to have it as well. I will see to make a tutorial in the near future.