lettucecfd / lettuce

Computational Fluid Dynamics based on PyTorch and the Lattice Boltzmann Method
MIT License
223 stars 39 forks source link

wrong order of basic LBM steps? #103

Closed MartinKliemank closed 2 years ago

MartinKliemank commented 2 years ago

According to the diagram in Krüger page 67 (Fig. 3.2) "An overview of one cycle of the LB algorithm" the proper order of algorithm steps is:

increment time > output > collision > propagation > boundaries

however in lettuce we implemented:

self.i += 1
self.f = self.streaming(self.f)
self.f = torch.where(self.no_collision_mask, self.f, self.collision(self.f))
for boundary in self._boundaries:
  self.f = boundary(self.f)
self._report()

so increment time > propagation > collision > boundaries > output

which doesn't agree, so I think it means that the result of boundaries is streamed away instead of replacing the streaming step for the respective fs (because of the no stream masks they wont be overwritten but will instead be on both n and n-1 nodes)# also I think we are reporting a state where the time step isn't fully over

Please have a look and see if we should fix this @Olllom @McBs @dominikwilde

Olllom commented 2 years ago

I think you're right. The wrong sorting of those operations (stream, collide, boundary, report) means that the temporal convergence near the boundaries is probably linear instead of quadratic in the current code.

It's something that might be able to test wrt. an analytic solution using an unsteady Poiseuille flow driven by a body force.

If we want to be rigorous, this should probably be part of our convergence test suite. But I would be comfortable changing it without including this test. Feel free to open a PR. Or do you have any concerns, @dominikwilde / @McBs?

McBs commented 2 years ago

No concerns. I have already recommended creating this Issue and a PR. It might be fixed quickly since this is only a sequence of operations. We can use the convergence test for further students as a starter task to get into the LBM and Lettuce ;-).

MartinKliemank commented 2 years ago

That should be everything then I think?

The proper verification sadly sounds like a lot of work again :(^^

MartinKliemank commented 2 years ago

/push wasn't this fixed in #114? @Olllom (just to make sure I / someone can close it)

Olllom commented 2 years ago

yes, fixed in #114