FiniteVolumeTransportPhenomena / PyFVTool

Finite volume toolbox in Python
GNU Lesser General Public License v2.1
13 stars 4 forks source link

Terms for SphericalGrid1D not implemented #2

Closed mhvwerts closed 4 weeks ago

mhvwerts commented 1 year ago

This is not very urgent. I am trying out PyFVTool and it seems that the 1D spherical mesh has not been implemented yet.

I was trying to port the example concerning 1D spherical diffusion from the original Matlab FVTool to PyFVTool, and found that the createMeshSpherical1D does not exist (yet).

For now, I will go play in a different coordinate system. Anyway, thanks for PyFVTool which is an interesting option for using the finite-volume method in teaching transport phenomena and for setting up toy(?) models for research.

simulkade commented 1 month ago

I spent my last hours of holidays on implementing most of the spherical terms for 1D and 3D in a separate branch. It still needs to be tested, e.g. with @mhvwerts own example that was previously done in FVTool here. Feel free to have a look @mhvwerts. Since it is -kind of- linked to an ongoing project of ours at DTU, I will try to spend some work hours on it this week.

simulkade commented 1 month ago

With the latest commit, all .py tests pass (I could not get the notebook tests to work on my machine; not a priority at the moment). The diffusion example seems to give reasonable results although not yet compared with an analytical solution. The visualization is also broken for 3D spherical grid. I am in favor of drawing several circular wedges either in parallel or perpendicular. However, for incomplete spheres it is not the best option.

simulkade commented 1 month ago

For now, one must avoid $\theta = \pi$ when creating a 3D spherical grid.

mhvwerts commented 1 month ago

It is great that you are implementing the terms for spherical coordinates (1D... and even 3D). I am eager to take a closer look and hope to do that somewhere in the coming days.

Indeed, the notebook tests are not easy to run and need a special conda environment to make pytest-notebook happy. I am planning to export the code from the notebooks to conventional test scripts so that they run with vanilla pytest. We would then simply keep the notebooks as documentation/examples and have separate test scripts running the same code. The notebook examples may then evolve, while the (more basic) test scripts can stay the same.

mhvwerts commented 1 month ago

There are some additional subtleties with notebook-based testing:

This is quite a hassle, I realize. I will take care of occasional notebook testing for now, and keep in mind to create stand-alone conventional test scripts.

mhvwerts commented 1 month ago

I pushed some small changes that went directly to your "pyfvtool-spherical" branch. Don't forget to pull them in to your local branch. Later on, I will probably work again on a separate branch in my fork and file pull requests as necessary.

mhvwerts commented 1 month ago

Anyways, notebook testing is working for me now and all tests pass, so your new code also runs on my computer. I hope to get back to this soon (the coming days?).

simulkade commented 1 month ago

Thanks, @mhvwerts. Just pulled all your changes. I am struggling a bit with the boundary conditions at the north and south poles of the spherical grid. Otherwise, everything is fine.

simulkade commented 1 month ago

I also forgot that the range for $\theta$ is [0, $\pi$]. I fix the warning message.

mhvwerts commented 1 month ago

The 1D spherical diffusion example is working. There is a notebook illustrating this. Nice to see this working with PyFVTool!

simulkade commented 1 month ago

This looks great! I will clean up and make a pull request later today.