marchirschvogel / ambit

FEniCSx-based cardiovascular multi-physics solver
MIT License
33 stars 7 forks source link

[JOSS Review] Improve documentation #6

Closed CZHU20 closed 11 months ago

CZHU20 commented 1 year ago

This is a very nice package, and thanks for making it open-source! I would like to suggest the following additions to the documentation:

  1. Add examples and their documentation. It seems to me that tests also serve as examples now. I think there are differences between these two. Examples, especially those non-trivial ones derived from real applications, serve the purpose of showcasing the capability of the software. I would recommend adding one example for each highlighted capability (fluid, solid, 0D, coupling, etc.) and provide documentation for them (problems solved, expected solutions, etc.).

  2. Add community guidelines: JOSS requires clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support.

marchirschvogel commented 11 months ago

Thank you for bringing this to my attention.

  1. Indeed, examples that walk a user through the basic problem types were missing, and understanding everything from the tests was not feasible (especially since only fractions of the setups were solved in the tests). Therefore, I added examples for solid mechanics (standard nonlinear elasticity), fluid dynamics (incompressible Navier-Stokes), solid as well as fluid coupled to 0D flow models (demonstrating how to simulate a heart beat and a "valve bypass"), and how to use pure 0D models to assess cardiovascular quantities. The examples incl. results plots are in the folder in demos. By no means they cover all of the code, but give a user a good starting point for the respective problems. Feel free to run them and check whether their documentation is sufficient for understanding what is happening.

  2. There is a file CONTRIBUTING.md in the top folder structure, which I believe covers already the aspects you mentioned. Please let me know if this is not sufficient such that I can add any further things.

CZHU20 commented 11 months ago

The demos look really great. I was able to run all cases. There is only one issue. Cases fluid and fluid_flow0d can only be run with ghcr.io/marchirschvogel/ambit:devenv, which is not stated in the README of fluid.

marchirschvogel commented 11 months ago

Thanks for pointing to this. I've updated the latest Docker container now so that the fluid demo also runs there. Only the fluid_flow0d demo currently only runs inside the devenv container, because a special dolfinx branch is needed. I've added this in the demo description (which now has moved for all the demos to the docs, https://ambit.readthedocs.io/en/latest/documentation.html#demos). One of the dolfinx developers confirmed that the special 'mixed' branch will soon be merged into main, so I hope this distinction will become obsolete for my code setup soon.