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

Update gravitational constant #4140

Merged
merged 2 commits into from Jul 9, 2021
Merged

Conversation

pmbremner
Copy link
Contributor

Updated the gravitational constant to match the recommended value from CODATA.

The published value (with standard error in parenthesis) and the reference were added to the comments.

This change aids gravity comparisons of benchmarks with published studies.

Pull Request Checklist. Please read and check each box with an X. Delete any part not applicable. Ask on the forum if you need help with any step.

Describe what you did in this PR and why you did it.

Before your first pull request:

For all pull requests:

Updated the gravitational constant to match the recommended value from CODATA.

The published value (with standard error in parenthesis) and the reference were added to the comments.

This change aids gravity comparisons of benchmarks with published studies.
Copy link
Contributor

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

I would love us to be able to run simulations where this is the limiting factor on the accuracy of results :-)

@bangerth
Copy link
Contributor

bangerth commented Jul 8, 2021

/rebuild

@tjhei
Copy link
Member

tjhei commented Jul 8, 2021

Well, it does matter for a few tests, apparently. 😉

@bangerth
Copy link
Contributor

bangerth commented Jul 8, 2021

It turns out that a number of tests are apparently sensitive enough to actually fail now:

	241 - dynamic_core (Failed)
	278 - geoid (Failed)
	279 - geoid_no_topo (Failed)
	280 - geoid_visualization (Failed)
	302 - gravity_point_values_list (Failed)
	303 - gravity_point_values_map (Failed)
	304 - gravity_point_values_spiral (Failed)
	716 - spectral_comparison (Failed)

Would you mind updating the test outputs?

@pmbremner
Copy link
Contributor Author

I'll start working on these

@pmbremner
Copy link
Contributor Author

The above commits should cover all the files from the failed tests

@bangerth
Copy link
Contributor

bangerth commented Jul 8, 2021

Let's see what the tester says. I only spot checked, but is your sense also that the changes are marginal and likely correct?

@pmbremner
Copy link
Contributor Author

I believe they are correct. Some values differ by more then a marginal amount, though, such as sin and cos coefficients. But summary values were the same or nearly the same, such as CMB heat flux in dynamic_core

@bangerth
Copy link
Contributor

bangerth commented Jul 8, 2021

OK, good to merge once the checks come back green!

@pmbremner
Copy link
Contributor Author

Hmm, are the tests still failing? I must need to do something more

@tjhei
Copy link
Member

tjhei commented Jul 8, 2021

Did you apply changes-test-results.diff as explained in the pdf manual?

@pmbremner
Copy link
Contributor Author

The diff file has now been applied. Rene helped me apply it and to squash the commits. Hopefully this fixes it

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Looks good now.

@gassmoeller gassmoeller merged commit 9d58beb into geodynamics:master Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants