Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selenopotential to degree and order 50 #2032

Merged
merged 18 commits into from
Dec 28, 2018

Conversation

eggrobin
Copy link
Member

Bump max_geopotential_degree and extend the numerical tables accordingly.

@eggrobin
Copy link
Member Author

The slow build is because of slow tests, specifically:

O  36 [       OK ] MercuryPerihelionTest.Year1960 (1075903 ms)
O  36 [----------] 1 test from MercuryPerihelionTest (1075903 ms total)
O  36 [==========] 1 test from 1 test case ran. (1075906 ms total)
O 760 [       OK ] BodyTest.SolarNoon (1171020 ms)
O 760 [----------] 1 test from BodyTest (1171020 ms total)
O 760 [==========] 1 test from 1 test case ran. (1171020 ms total)
O  15 [  FAILED  ] GeodesyTest.LAGEOS2 (1916935 ms)
O  15 [----------] 1 test from GeodesyTest (1916935 ms total)
O  15 [==========] 1 test from 1 test case ran. (1916935 ms total)
Done (2039077 ms)

Maybe we should not use a geopotential tolerance of 0 for those...

@eggrobin
Copy link
Member Author

With the new geopotential tolerances, the tests take as long as they used to, but the build still takes more than 30 min; we can't merge this as-is.

@eggrobin eggrobin closed this Dec 23, 2018
@eggrobin eggrobin reopened this Dec 24, 2018
@eggrobin eggrobin closed this Dec 24, 2018
@eggrobin eggrobin reopened this Dec 24, 2018
@eggrobin
Copy link
Member Author

For future reference: the detemplatization of DegreeNOrderM is at https://github.com/eggrobin/Principia/tree/detemplatization.

astronomy/sol_gravity_model.proto.txt Outdated Show resolved Hide resolved
geopotential {
row {
degree: 2
column {order: 0, cos: -9.0884339347424299e-05, sin: 0.0000000000000000e+00},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use 0 for the zeroes, like we did for the Earth?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

astronomy/sol_gravity_model.proto.txt Show resolved Hide resolved
@pleroy pleroy added the LGTM label Dec 28, 2018
@eggrobin
Copy link
Member Author

Benchmarking: the loss of the FORCE_INLINE doesn't change much, it just gets slower regardless, see https://gist.github.com/eggrobin/3453c16bf67f9a99f3e24c57722eeeda/revisions.

@eggrobin eggrobin merged commit 05ab936 into mockingbirdnest:master Dec 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants