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

Code interpretability #21

Open
martinjanssens opened this issue Feb 26, 2022 · 7 comments
Open

Code interpretability #21

martinjanssens opened this issue Feb 26, 2022 · 7 comments

Comments

@martinjanssens
Copy link

Opening this as a place to collect loose thoughts on the ease with which the source code can be read, understood and worked with.

@martinjanssens
Copy link
Author

martinjanssens commented Feb 26, 2022

Starting from the top, I guess that most users that just naively want to know what the code does, would open MicroHH.jl and try to figure out what happens from there. That file, to me, would ideally serve as a table of contents for the rest of the code. Put differently, I'd like to open that file and immediately see the flow of the program, and where the different modules of the code are called in that main flow.

I think it already does this really well. You could consider:

  • moving the I/O stuff out of this file
  • explicit comments about what each include statement goes and gets (so if I want to know where advection is being calculated, I can actually see that include("Dynamics.jl") gets calc_dynamics_tend, which is then used in the main loop)
  • giving step_model a more central place, e.g. at the very top after you've defined the model.
  • order the 'main' functions so that they come in 'chronological' order, i.e. something like:
  1. function Model
  2. function prepare_model
  3. function step_model
  4. function calc_rhs
  5. Other supporting functions. Maybe this is not possible if the supporting functions need to be defined before they can be called (I don't know if this is the case in Julia?), but you get the idea.

@martinjanssens
Copy link
Author

The structs (Fields, Grid...) are a great choice to pass information around. It very pleasant to read things like m.fields[i].u; this works very well.

@martinjanssens
Copy link
Author

martinjanssens commented Feb 26, 2022

The passing around of the Parallel may feel a little cumbersome, but it actually reads quite well. It's very explicit now when a function needs to account for the parallelisation, even if just to print. Making this global is something to consider, but I wouldn't say it's broken at the moment. Also, if you want to add global classes, I'd be very happy to see those defined immediately and very obviously, so I'd know just from looking at initial MicroHH.lj that 'ah, these are available everywhere'.

@martinjanssens
Copy link
Author

martinjanssens commented Feb 26, 2022

StencilBuilder.jl is very nice, but rather abstract. So if I wanted to implement my own advection scheme for instance, I really would have trouble starting with that. Prefacing this file with a little comment on how the stencil builder works would be really useful. In general, you could consider having documentation on a per file basis, where you spend a few sentences just explaining the general idea and contents of each file. This would especially be useful for the more complex modules (Pressure.jl is another good example).

@martinjanssens
Copy link
Author

I was also wondering why you would gather all the dynamics in a single expression. It somehow feels more intuitive to have advection in one file, diffusion in another, i.e. just one file for each process on the RHS of the equations. That would presumably make it easier to integrate an SFS model in the future, e.g.. This is a really small point, of course.

@leifdenby
Copy link

StencilBuilder.jl is very nice, but rather abstract. So if I wanted to implement my own advection scheme for instance, I really would have trouble starting with that. Prefacing this file with a little comment on how the stencil builder works would be really useful. In general, you could consider having documentation on a per file basis, where you spend a few sentences just explaining the general idea and contents of each file.

I was wondering if you'd considered using https://github.com/eth-cscs/ImplicitGlobalGrid.jl for the underlying grid datastructure and stencil operations on it?

This is really cool stuff @Chiil !

@Chiil
Copy link
Owner

Chiil commented Jun 18, 2022

@leifdenby Thanks, Leif. It is more or less a small-scale hobby project. Julia has some cool features that makes life a lot simpler with respect to C++. I did not know this package, I will have a look!

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

No branches or pull requests

3 participants