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

Kinetic energy fraction #329

Merged
merged 7 commits into from
Oct 28, 2024
Merged

Kinetic energy fraction #329

merged 7 commits into from
Oct 28, 2024

Conversation

benoitlaurent96
Copy link
Collaborator

I added a function in Hydrodynamics and HydrodynamicsTemplateModel to compute the efficiency factor for a given wall velocity. I also made a test to compare the results obtained with both methods.
I get a DeprecationWarning in the test, but I really don't know why. Pytest doesn't give much detail about it and it only appears in the test, so I'm not sure what to do about that.

Copy link
Collaborator

@jorindevandevis jorindevandevis left a comment

Choose a reason for hiding this comment

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

Great work, Benoit! It looks good and I also added an additional test, comparing to my other code, and it passes.

I am just wondering if it is not more practical for the reader to just return the energy budget rather than the efficiency factor. That would just mean we multiply the efficiencyFactor with 3/4 * alpha *enthalpy/energy.
I think this is more practical, as the energy budget goes into the formula for gravitational waves. The efficiencyFactor still has to be converted and that is a possible source for confusion.

Could you also add a formula in the doc string?

@benoitlaurent96
Copy link
Collaborator Author

I was a bit unsure about how to compute K because the energy, which enters in its calculation, is not always well-defined since you can always add a constant to it. If I understand correctly, the usual convention is to set the vacuum energy to 0 inside the bubble. But with a general potential, there is not always a clear separation between the vacuum and thermal energy, so I don't know what it means to set the vacuum energy to 0. Do you know a better definition?

@jorindevandevis
Copy link
Collaborator

That is a good point, I hadn't thought of it. But indeed, that's also the only definition that I know. OK, then I think it's best to leave the responsibility with the user.

@benoitlaurent96
Copy link
Collaborator Author

Agreed!
And I added the formula in the doc string.

@benoitlaurent96 benoitlaurent96 merged commit d0d151f into main Oct 28, 2024
3 checks passed
@benoitlaurent96 benoitlaurent96 deleted the kineticEnergyFraction branch October 28, 2024 19:22
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