DavidBrust / MultiComponentReactiveMixtureProject

Project repository for model implementation of multi-component transport of reactive gas phase mixtures in porous media.
MIT License
2 stars 0 forks source link

Testing the code #4

Open j-fu opened 1 month ago

j-fu commented 1 month ago

Hi, I am just testing the code. I am missing the file data/Goell2012/Loschmidt/xAr_Bottom.csv

Will make a pull request with some other fixes.

DavidBrust commented 1 month ago

Hi, thanks for taking a look.

When you are on it, could you check, that the "boundary flux functions", for the boundary species is working as intended? I defined boundary species with associated boundary reaction terms and boundary fluxes .

While the boundary reaction terms appear to work fine, I have a suspicion, that something is not quite right with the boundary fluxes. I would expect, that the diffusive boundary flux terms should smoothen out differences in the solution along the boundary, which appears to not be the case, e.g. see PTReactorDemo.jl, when running the 2D case.

j-fu commented 1 month ago

Yes, I will check this. Meanwhile I an trying to run the script, and I don't exactly understand what this line: https://github.com/DavidBrust/MultiComponentReactiveMixtureProject/blob/b4a542ef6f41e905bb4e8773b18947bd47529343/scripts/PTR3D.jl#L77 does. I suspect it is just for changing the aspect ratio of the grid for visualization purposes, but in the moment it creates a grid with more than 21_000_000 nodes. If my assumption is true, the better way would be creating a grid from the components of the old one just with scaled z coordinate.

DavidBrust commented 1 month ago

Hi, Yes, your assumption is correct, it is for later visualization in ParaView. I must have forgotten about this in the main branch, I have been working with the branch "block_domain_partially" where I am using this (alongside some more modifications to the domain and domain boundaries via PTR_grid_boundaries_regions!): https://github.com/DavidBrust/MultiComponentReactiveMixtureProject/blob/f85f9a83e81988bd7520911e8184498c89ca5302/scripts/PTR3D.jl#L368

Please feel free to change the line you referenced on the main branch as you have proposed.

j-fu commented 1 month ago

Hi, thanks for taking a look.

When you are on it, could you check, that the "boundary flux functions", for the boundary species is working as intended? I defined boundary species with associated boundary reaction terms and boundary fluxes .

While the boundary reaction terms appear to work fine, I have a suspicion, that something is not quite right with the boundary fluxes. I would expect, that the diffusive boundary flux terms should smoothen out differences in the solution along the boundary, which appears to not be the case, e.g. see PTReactorDemo.jl, when running the 2D case.

I had a look but at the first glance I did not find the (a) problem. May be I need more input on this.

j-fu commented 1 month ago

And see #5

j-fu commented 1 month ago

I see some more possible improvements, e.g. the use of DrWatson.jl for data file name generation. I also may have a look at the tests, as I would like to use the code for downstream tests.

I plan a breaking change for VoronoiFVM concerning the specfication of linear solvers, but this will come in a couple of weeks. I would use among others the PTR code for benchmarking this.

DavidBrust commented 1 month ago

Thanks for the improvements in the pull request, I have merged it into the main branch.

I had a look but at the first glance I did not find the (a) problem. May be I need more input on this.

I will take a look and try to create an example where the behavior I felt was unexpected shows up. (It is possible that everything works and my expectations are off. I will double check.)