tum-pbs / PhiFlow

A differentiable PDE solving framework for machine learning
MIT License
1.39k stars 189 forks source link

Extrapolations get added when adding fields #153

Closed KarlisFre closed 6 months ago

KarlisFre commented 6 months ago

Hi there is confusing behavior on fields having extrapolations. See this code:

from phi.flow import *

smoke = CenteredGrid(0, extrapolation.combine_sides(x=(1, 0), y=ZERO_GRADIENT), x=100, y=100, bounds=Box(x=100, y=100))
zero_field = CenteredGrid(0, ZERO_GRADIENT, x=100, y=100, bounds=Box(x=100, y=100))
print(smoke.extrapolation)
smoke = smoke + zero_field @ smoke # I expected this operation would do nothing
print(smoke.extrapolation) # but the extrapolation has changed !!!

I expected that the resampling and adding operation should do nothing since I add zeroes, but this is not the case and extrapolation changes:

{('x', False): 1, ('x', True): 0, ('y', False): zero-gradient, ('y', True): zero-gradient}
{('x', False): 2, ('x', True): 0, ('y', False): zero-gradient, ('y', True): zero-gradient}

I tried to bypass it by using: smoke = smoke + field.resample(zero_field, smoke, keep_extrapolation=True) # crash but this line crashes: TypeError: unsupported operand type(s) for +: 'ConstantExtrapolation' and '_ZeroGradient'

Maybe the desired behavior would be not summing the extrapolations?

holl- commented 6 months ago

Right, the @ gets applied first, which returns zeros with the boundary condition of smoke, then the + adds the boundary conditions. If you want to keep the zero_field boundary condition, use

smoke = smoke + zero_field.at(smoke, keep_extrapolation=True)

However, this will not work with the current BCs because ZERO_GRADIENT and 1 are not compatible.

KarlisFre commented 6 months ago

I solved this by constructing a new grid having the value which is the sum of both values and with the right extrapolation.

KarlisFre commented 6 months ago

There is another example where boundary conditions are treated wrong:

velocity = StaggeredGrid((0,0), extrapolation.combine_sides(x=(1, 0), y=ZERO_GRADIENT), x=100, y=100, bounds=Box(x=100, y=100))
OBSTACLE = Obstacle(Sphere(x=50.5, y=60, radius=10), velocity=(1,1))
print(velocity.extrapolation)
velocity = fluid.apply_boundary_conditions(velocity, [OBSTACLE])
print(velocity.extrapolation)

Extrapolation is changed in apply_boundary_conditions:

{('x', False): 1, ('x', True): 0, ('y', False): zero-gradient, ('y', True): zero-gradient}
{('x', False): 2, ('x', True): 0, ('y', False): zero-gradient, ('y', True): zero-gradient}
holl- commented 6 months ago

Oh right. I'll fix that in the 3.0 branch soon.

holl- commented 6 months ago

Should work in 3.0 now.

pip install --upgrade git+https://github.com/tum-pbs/PhiFlow@3.0