Remove hardcoded v_0 from earth_velocity

[JoranAngevaare]

Main change

• In this PR, we update earth_velocity to use v_0 from the StandardHaloModel-class instead of hardcoding it.
• Change the default of StandardHaloModel to v_0 = 238 km/s following the whitepaper
• Change v_orbit, v_pec to the white paper (small changes)

We used this table for the values:

Other changes

We also had to update the reference values as v_earth was always returning 232 km/s even if v_0 doesn't really match up with that. Instead, we now chose a time t=59.37 which gives the same value as the annual average for v_0=238 km/s

Please notice that this description was updated after the discussion below

[JelleAalbers]

Thanks for these fixes and changes Joran!

In this PR, we update earth_velocity to use v_0 from the StandardHaloModel-class instead of hardcoding it.

Thanks for spotting this: observed_speed_dist wasn't correctly passing its v_0 argument along. Glad this is fixed, so overriding v_0 now actually works..

Change the default of StandardHaloModel to v_0 = 238 km/s following the whitepaper

That's a big change, but certainly justified. We'll clearly have to mark this with a new release!

We also had to update the reference values as v_earth was always returning 232 km/s

232 km/s is a historical convention dating back to at least before 2002 (https://arxiv.org/pdf/hep-ph/0504010.pdf). You're right it's not the average, maybe they picked something a bit lower to be conservative? In any case, since we're changing v_0 to follow the whitepaper, I agree we should also change v_earth.

Instead of actually averaging the v_earth and caching the result, how would you feel about picking a reference time to use if none is provided? t=50.66 (roughly February 20) would reproduce 232 km/s with the old v_0; t=78 (March 18) would get us the annual average speed (at least with the old v_0).

I guess averaging the WIMP energy distribution over time would be the ultimate sensible default, but that's probably a bigger task / out of scope of wimprates / too big a behavior change (it would slow the default calls down significantly).

To illustrate this point, in 25c1317, we added a test ..

Thanks for the illustration, but unless I'm mistaken the test is still in the build suite, so it's causing red crosses in your PR checks :-)

We might consider doing something similar for v_pec etc, using the very slightly updated variables from this paper: https://arxiv.org/abs/2105.00599

Absolutely. If we're changing v_0 anyway would be great to switch to the official params in the same release.

[JoranAngevaare]

Thanks Jelle! I like the idea of just picking one t= day.

Instead of actually averaging the v_earth and caching the result, how would you feel about picking a reference time to use if none is provided? t=50.66 (roughly February 20) would reproduce 232 km/s with the old v_0; t=78 (March 18) would get us the annual average speed (at least with the old v_0).

Okay, maybe I did something odd, but for v_0 = 238 I got that t=59.37 gives the right v_earth. Please see the test in the tests/test_wimprates.py

[JoranAngevaare]

@JelleAalbers what do you think, should we make a few edits and merge?

[JelleAalbers]

Looks good Joran, thanks! I agree on removing that one funny comparison -- feel free to do that and merge this, or just wait a few days for me to not be lazy and do it myself (then I'll merge and make a new minor release).

[JoranAngevaare]

Thanks Jelle, I removed the messy if statement. Please let me know if I could do some additional checks, otherwise it seems ready from my side.

[JelleAalbers]

Thanks Joran! I'll make an issue for the compatibility with newer numpy versions.