Set interior structure by: total planet mass, radius #272
+472
−447
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… components. Can optionally set it by radius in config. Generalised structure wrapper somewhat
| # This function takes R_int as the input value, and returns the mass residual | ||
| def _resid(x): | ||
| hf_row["R_int"] = x | ||
|
|
||
| log.debug("Try R = %.2e m = %.3f R_earth"%(x,x/R_earth)) | ||
|
|
||
| # Use interior model to get dry mass from radius | ||
| run_interior(dirs, config, IC_INTERIOR, hf_all, hf_row, verbose=False) | ||
| if solve_g: |
There was a problem hiding this comment.
Is there a case in which solve_g would be meaningfully turned off?
There was a problem hiding this comment.
No, I don't think so. Leaving it set to true works entirely fine for both Dummy and SPIDER, and will update the surface gravity self-consistently with mass and radius of the interior.
The reason this flag is here is because Aragog will crash when it is set to true - I think related to this issue with density that @planetmariana is working on. We should try to remove this as soon as possible.
tests/data/integration/physical/plot_population_time_density.png
Outdated
Show resolved
Hide resolved
tests/data/integration/physical/plot_population_mass_radius.png
Outdated
Show resolved
Hide resolved
timlichtenberg
requested changes
Nov 19, 2024
timlichtenberg
approved these changes
Nov 19, 2024
|
Thanks a lot, @timlichtenberg! We should definitely look into this structure stuff more deeply, but I think that this is better than the previous implementation of over-specifying the mass and radius. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reworks the methods by which the user can set the interior structure. Previously, they would specify the dry interior mass, and the interior models would be used to inversely solve for the interior radius.
This is now offset by the total volatile inventory, such that the user only has to specify the total planet mass. This is what would be probed by RV or TTV measurements, so it is much easier to specify for the user. Closes #269.
Additionally, the user can avoid the whole mass -> radius conversion by setting the interior radius directly in the configuration file. Closes #267.
The physical test (which uses Aragog) now sets the structure using the planet radius (hence the test updates). The dummy test (which uses the Dummy interior) sets the structure using the total planet mass.
Minor things:
reset_vmrflag with the more obviousrainoutflag. This PR includes a minor change to handle this accordingly.