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

Changes to lift the dependence of turb on star_data #739

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

rhdtownsend
Copy link
Contributor

While making some enhancements to the thermohaline code (which is part of the turb module), I discovered a bad code smell: the turb module depends on the star_data module. This doesn't make architectural sense; turb handles macrophysics, and shouldn't know anything about stars.

The dependency comes through turb/private/tdc_support.f90; the convert() and unconvert() functions need to know how many variables are stored in auto_diff_real_star_order1%d1Array. This number is currently stored in the auto_diff_star_num_vars parameter defined in star_data/public/star_data_def.inc.

A functionally equivalent way to do this, which also fixes the architectural sense, is to use the actual declared length of auto_diff_real_star_order1%d1Array rather than auto_diff_star_num_vars.

This PR makes this change, plus a few others that accompany it:

  • Move star_data from the LIBS_MESA_MICRO definition in utils/makefile_header (where it shouldn't be) to the LIBS_MESA_STAR_SUPPORT definition
  • Rearrange the build order in ./install so that turb builds just after atm, and star_data builds after all the micro/macrophysics modules (turb included).

I'd appreciate a quick turnaround on the PR, as it's needed in order for the new thermohaline stuff to work.

Copy link
Member

@pmocz pmocz left a comment

Choose a reason for hiding this comment

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

@rhdtownsend rhdtownsend merged commit 427c21b into main Oct 8, 2024
3 checks passed
@rhdtownsend rhdtownsend deleted the remove_bogus_turb_dependence branch October 8, 2024 13:23
warrickball added a commit that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants