Efficiency - Githubissues

danieljfarrell / FVM

Finite volume method applied to the advection-diffusion equation

BSD 3-Clause "New" or "Revised" License

37 stars 16 forks source link

Efficiency #1

Closed kookma closed 5 years ago

kookma commented 5 years ago

Hi Daniel, I went through the code and I see there may be rooms for efficiency. By the way I have some questions:

Why the the model iteself creates a, d, k while you have created them already in the example code?
Is there any reason the solver is not implemented inside Model calss?
Why not to include the source term?

kookma commented 5 years ago

More input: What do you think if we could use pointer for storing the mesh data?

danieljfarrell commented 5 years ago

Hi Mohammad,

It’s been quite a while since I wrote this, 6 years! But it seems fairly efficient and clean to me. The CellVariable is a subclass of numpy.ndarray and takes a view of the array. That is, it uses the same memory, so there is no duplication. It is basically parsing the data by reference (or my pointer as you say), python does not have the concept of pointers.

I didn’t follow the other points you mentioned.

Feel free to take this code and improve upon it and refactor it. Write some benchmarks etc.

I wrote this to teach myself about the numerical methods behind finite volume method. It might not be test best code. But it served it’s purpose for me.

kookma commented 5 years ago

Hi Daniel, Thank you for your prompt reply.

You are right, I understood FVM parses the data by reference, so there is no penalty in this case and it uses memory here effectively.
I will go through the deeper in code. I am following the documentation and I now better understand what you did in code
By source term I mean to solve ut = -aux + duxx +s(x,t,u) The last is the source term.

By the way thank you for your great code. I like the OOP concept you have used in this code. I gonna to change it for multi-component multi-reaction advection-diffusion-reaction problems.

Just one more question: Does the code in the current state can solve the set of PDEs (few coupled PDEs)?

Best Mohammad

danieljfarrell commented 5 years ago

Why not to include the source term?

With this project, I was interested in better understanding how to write the derivative terms as matrices, generalize those to different types of boundary conditions and provide a reasonable API for constructing systems. As the source term does not have a derivative I didn't focus on that.

Does the code in the current state can solve the set of PDEs (few coupled PDEs)?

I really would not advise you to use this code for anything other than learning. There are plenty of other projects such as FiPy or FEniCS that are real solvers. To solve coupled equations you would need to construct block matrices.

I'm going to close this issue now.

kookma commented 5 years ago

Does the code in the current state can solve the set of PDEs (few coupled PDEs)?

I really would not advise you to use this code for anything other than learning. There are plenty of other projects such as FiPy or FEniCS that are real solvers. To solve coupled equations you would need to construct block matrices.

Thanks Daniel. Yes I know this is for learning purpose and I was going to use it as a case study for numerical methods class. Those real solvers are complicated and not suitable for pedagogical purpose..

By the way, I understood this code worth enough to be used as homework in the class as a linear and single equation PDE solver.

I'm going to close this issue now.

No problem.

Thanks again