Closed JoyMonteiro closed 7 years ago
Merging #1 into master will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #1 +/- ##
=======================================
Coverage 86.38% 86.38%
=======================================
Files 12 12
Lines 661 661
=======================================
Hits 571 571
Misses 90 90
Impacted Files | Coverage Δ | |
---|---|---|
sympl/_core/constants.py | 100% <ø> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8badfac...ca31609. Read the comment docs.
Should there be a seconds_per_day constant, when there is already a planetary_rotation_rate constant (in days/rotations per second)? I would think not, since a user would expect modifying one to modify the other, but that's not the case unless we do some fancy dictionary subclassing on the default_constants dictionary. Other than that everything looks good.
2pi/planetary_rotation_rate does not give the expected 86400 seconds, which is apparently something called a mean solar day*. Deriving duration of day from planetary rotation seems quite involved because of eccentricity and obliquity: https://en.wikipedia.org/wiki/Equation_of_time
Therefore, it might help to have it around.
The correct expression is 2 x pi/planetary_rotation_rate x 365.25/364.25 (to account for Earth rotating around the sun, since planetary rotation rate is in an inertial frame of reference), and this gives a very close 86400.71 seconds if you use a more precise value of 7.29211e-5 radians/second for the planetary rotation rate (almost accurate up to the number of sig figs).
But I suppose I'm fine with keeping these separate. We should perhaps document that seconds_per_day is to be used to determine exactly that, while rotation_rate is to be used for dynamical computations and not to determine length of day (since you would need an additional parameter, length of year, to determine it).
Can you revise HISTORY.rst with these changes, including putting the change to stefan_boltzmann name in breaking changes?
Great, thanks. I think eventually I might write a utility function which takes orbital parameters and determines mean length of day, but this will help in the short term.
I will revise HISTORY and add it to this PR in the morning.
Merging with PR #2.
Added some new constants, taken from the NIST database. Also, changed the name of SB to add constant at the end, to make it consistent with all other "constants".