igmk / pamtra

Passive and Active Microwave TRAnsfer model
GNU General Public License v3.0
20 stars 16 forks source link

Vertical wind and particle velocity #26

Closed markuskarrer closed 4 years ago

markuskarrer commented 4 years ago

The MDV did not fit to the vertical wind speed (W) in regions where W is larger than the terminal velocity.

image image

The reason for this seems to be, that the radar_spectrum.f90 W was added to velspec instead of subtracted.

Does this raise issues with radar_airmotion_step_vmin and radar_airmotion_step_vmax?

maahn commented 4 years ago

Hi Markus, the sign of the air motion follows the same sign convention as the rest of PAMTRA: down is positive. To be more precise, this is the convention of the fall velocity relations, the rest of PAMTRA doesn’t care. This means model output needs to be multiplied with -1.

I admit that this is poorly documented, but I would not change it to avoid breaking old code and also because I find it confusing if different conventions are used. So I would propose to revert a4725050dfdcde84bccf1302f1e6ad310b7dddfe and improve documentation instead, but I’m open to a better solution. Maybe adding a fall velocity convention switch?

DaveOri commented 4 years ago

I am not really sure this is the best. W is kind of universally defined as vertical wind speed and it only makes sense to be upward positive if Z-axis is oriented upwards. I also do not like that it has a different convention with respect to fallspeeds, but somehow I can understand better that fallspeeds are positive even though their velocity vector is opposite of Z. If we keep wind_w with the same reference as dia2vel we need somehow to specify that wind_w is not vertical wind but it is the ... "downdraft speed", maybe?

maahn commented 4 years ago

We could leave Markus' commit in place and rename wind_w to wind_w_positiveup. This would break old code (which is better than changed results without any notice), but hopefully people would get the message and double check their signs when adapting the code.

DaveOri commented 4 years ago

I am not against reverting a472505 it is just that in this case we we will have to change the icon_meteogram importer (the only one that imports W) and results will change in any case for people that uses it (mostly me and perhaps Claudia ...). I see the value of not changing the general reference of having velocities going down. This would definitely help to maintain the code since developers will not have to change their way of thinking. My suggestion is to do like that:

markuskarrer commented 4 years ago

I will revert the commit and modify the importer. I think that is a good suggestion considering the violation of old code. But I think it is also crucial to inform users about the convention. It is very tempting to just pass all the model variables as they are to Pamtra. One really has to be aware of the different convention in PAMTRA compared to (probably all) models, in order to not make this error.

maahn commented 4 years ago

I assume this has been fixed with db1c32c5603203adf4e56e97e5ae7644f73be0bb