search

j-fu / VoronoiFVM.jl

Solution of nonlinear multiphysics partial differential equation systems using the Voronoi finite volume method
MIT License
183 stars 33 forks source link

Simplifiy data #6

Closed jw3126 closed 3 years ago

jw3126 commented 3 years ago

Thanks for creating this package! I like that this package allows to solve equations in few lines without much boilerplate. I think it can be made even more lightweight by getting rid of the AbstractData type. In julia it is good practice to only impose type restrictions if you really need them. They add boilerplate while giving no static safety or performance. I also removed some files that were byte by byte duplicates.

j-fu commented 3 years ago

Hi,

thanks for the pull request. The duplicate tests have bee n due to the evolution of the code structure and the attempt to keep things compatible for those using it. As for the data - probably you are right. I will think about it. Anyway I come from c++, so the package design kind of reflects my learning history with Julia. I am currently on a holiday trip, will check things when I have a bit more time.

As for the discretization approach, what is the kind of upwinding you plan to choose ? I have some papers on this, including https://oar.tib.eu/jspui/bitstream/123456789/2481/1/871745119.pdf and https://hal.archives-ouvertes.fr/hal-02194604/ (we could move this discussion elsewhere, e.g. to an issue or email juergen.fuhrmann@wias-berlin.de)

jw3126 commented 3 years ago

Hi,

thanks for the pull request. The duplicate tests have bee n due to the evolution of the code structure and the attempt to keep things compatible for those using it.

So should I restore the files or are they no longer needed? Also, there are still some almost duplicate files, I only deleted the ones which were exactly the same.

I am currently on a holiday trip, will check things when I have a bit more time.

Sure, no hurry I wish you nice holidays!

As for the discretization approach, what is the kind of upwinding you plan to choose ? I have some papers on this, including https://oar.tib.eu/jspui/bitstream/123456789/2481/1/871745119.pdf and https://hal.archives-ouvertes.fr/hal-02194604/ (we could move this discussion elsewhere, e.g. to an issue or email juergen.fuhrmann@wias-berlin.de)

Thanks, you are right, let's not capture this PR for that discussion. Edit: I opened an issue #7

j-fu commented 3 years ago

Thanks for the PR, you are right, no need for AbstractData. Will indeed try to remove this in 0.9 ... As for the duplicate files, these have been example versions before a slightly API breaking change which still were supposed to run, but indeed it makes not much sense to have them.