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

Limiting convective velocities with max_conv_vel_div_csound #776

Closed
JRGarza opened this issue Feb 7, 2025 · 8 comments · Fixed by #780
Closed

Limiting convective velocities with max_conv_vel_div_csound #776

JRGarza opened this issue Feb 7, 2025 · 8 comments · Fixed by #780
Assignees
Labels
enhancement New feature or request tdc

Comments

@JRGarza
Copy link
Contributor

JRGarza commented Feb 7, 2025

Hi,
I noted the control max_conv_vel_div_csound is deffined in the documentation (for 23.05.1 and later versions), as well in ctrls_io, controls.defaults and star_controls but it is not used in the code. I notice this after trying to use the control to limit convection velocities below sound speed but did not see any difference, I am using Mihalas MLT option. Is there a reason for this, or it is just a remanent code from previous MESA versions?

@Debraheem Debraheem added the documentation Improvements or additions to documentation label Feb 7, 2025
@Debraheem
Copy link
Member

Thanks for opening this issue. Hmm, It looks like a remanent deprecated control, but it should be functional in theory, although I don't see it being used internally anywhere. It appears to be a topic of an open issue that was never addressed, so as of now there is no conv_vel limiter.

see #373

perhaps, we can revive it... but is it necessary?

For now, this control should either be removed, or put to use.

@Debraheem Debraheem self-assigned this Feb 9, 2025
@Debraheem
Copy link
Member

We are having git-lfs bandwidth issues at the moment, but when they are resolved, I can push a branch with a conv_vel limiter.

@Debraheem Debraheem added enhancement New feature or request tdc and removed documentation Improvements or additions to documentation labels Feb 9, 2025
@Debraheem
Copy link
Member

Perhaps you could git checkout this branch https://github.com/MESAHub/mesa/tree/EbF/add_Cs_limit_to_TDC? Let me know if you have any thoughts. I'm not immediately sure how to limit the conv_vel in mlt self-consistently like we do in TDC. The hack might have to cut it.

@JRGarza
Copy link
Contributor Author

JRGarza commented Feb 11, 2025

Thanks for the answer. For now I required the option for the 'Mihalas' option (as in my science case I have issues using TDC, but thanks for referring to the add_Cs_limit_to_TDC branch). I made a PR #781 for this control in case it is useful. As for the justification, the MLT prescriptions (at least Cox and Mihalas) are not valid for supersonic convection velocities. The argument in Cox and Giuli's book (chapter 14) states that turbulent pressure by non-supersonic convection is negligible and not taken into account for the derivation, contrary to the case of supersonic convection.

@Debraheem
Copy link
Member

Check out the the add_Cs_limit_to_TDC again. I added an option for mlt too. It seems similar to what you added?

On the opacity floor, that's a good idea, I've crashed many expanding models becauase the surface hits the opacity table boundary...

@Debraheem
Copy link
Member

I think the only difference is that you are using the actual sound speed at the cell face, where as I just used an estimate. I can correct that.

@JRGarza
Copy link
Contributor Author

JRGarza commented Feb 11, 2025

add_Cs_limit_to_TDC

I just saw the similarities between both. Sorry I thought that such branch was only usable for TDC. And it seems that the main difference between both is the csound considered for the limit as you mention.

@Debraheem
Copy link
Member

Yes, no worries! Happy to see we thought along the same lines. When I merge your branch into the add_Cs_limit_to_TDC branch, I'll squash the differences and pick one of our approaches.

@Debraheem Debraheem linked a pull request Feb 14, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tdc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants