Use correct fluid velocity in time extrapolation (Gizmo).

(cherry picked from commit 3970804b)

I think there is a mistake in the time extrapolation of the primitive quantities in the Gimzo and moving mesh hydro schemes. Just using the fluid velocity to calculate the time extrapolations from the spatial gradient violates Galilean invariance (pretty dramatically for very high bulk velocities). I think we should use the fluid velocity in the rest frame of the particle instead.

added Gizmo bug labels

requested review from @mivkov

assigned to @yuyttenhove

assigned to @matthieu and unassigned @yuyttenhove

The same mistake is also present in the computation ofdu_dt_old for the cooling, however in hydro_get_comoving_internal_energy_dt(...) we currently do not have access to the xpart to compute the relative fluid velocity.

Do you prefer changing the singature of hydro_get_comoving_internal_energy_dt or would you rather store the particle velocity in the hydro part itself (which is what I do for the moving mesh scheme). part.v is already something like that, but I think that velocity is extrapolated during the timestep, so it might not always be equal to the velocity at which the particle is drifted?

It's been a while since I took a close look at the frames we use everywhere. It'll take me a moment to double-check everything.

Do you have an example where you see the differences you mentioned?

This is the result of the sodshock test with a boost velocity of 500 with the current master:

And this is the same ICs with the above fix:

I have found that part.v is reset immediately after it has been drifted in hydro_gizmo_mfv_extra_velocity_drift (both in the MFV and MFM variants), so it will indeed always be the same to xpart.v_full and could be used to compute the relative fluid velocity for the computation of du_dt from the gradients (in hydro_get_comoving_internal_energy_dt).

while we're already at it, can we add the corresponding getter and setter functions, and try to consistently switch over to them?

Also, what exactly is the "rest frame of the particle"? Is that the frame of reference co-moving with the particle?

added 1 commit

• 590c2821 - Also use correct velocity in du_dt calculation + use getter and setter.

Compare with previous version

added 1 commit

• 267ba360 - Use relative velocity getter in timestep

Compare with previous version

Can we merge this?

Looks good to me. Sorry for the delay on my side.

resolved all threads

approved this merge request

Thanks for confirming and for the review.

merged

mentioned in commit 57ed8fe9