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

Add StressEnergyTensor free function #6457

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jyoo1042
Copy link
Contributor

@jyoo1042 jyoo1042 commented Jan 24, 2025

Proposed changes

Adding a function to evaluate the stress-energy tensor, which currently does not exist in StressEnergy.cpp
This will be mainly used for evaluating diagnostics for accretion disk simulations.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@jyoo1042 jyoo1042 requested a review from nilsdeppe January 24, 2025 21:51
@teukolsky
Copy link
Contributor

Can you please put in a comment about the purpose of this PR? What's new? Thanks.

Comment on lines 128 to 134
const auto rho_h_star =
(get(rest_mass_density) +
get(rest_mass_density) * get(specific_internal_energy)) +
get(pressure) + square(get(comoving_magnetic_field_magnitude));

const auto p_star =
get(pressure) + square(get(comoving_magnetic_field_magnitude)) / 2.;
Copy link
Member

Choose a reason for hiding this comment

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

You don't want auto here because this creates an expression tree that gets evaluated many times in the loop below. Specify the type

Comment on lines +40 to +57
void comoving_magnetic_field(
gsl::not_null<tnsr::A<DataType, 3>*> result,
const tnsr::I<DataType, 3>& spatial_velocity,
const tnsr::I<DataType, 3>& magnetic_field,
const Scalar<DataType>& magnetic_field_dot_spatial_velocity,
const Scalar<DataType>& lorentz_factor, const tnsr::I<DataType, 3>& shift,
const Scalar<DataType>& lapse);

template <typename DataType>
tnsr::A<DataType, 3> comoving_magnetic_field(
const tnsr::I<DataType, 3>& spatial_velocity,
const tnsr::I<DataType, 3>& magnetic_field,
const Scalar<DataType>& magnetic_field_dot_spatial_velocity,
const Scalar<DataType>& lorentz_factor, const tnsr::I<DataType, 3>& shift,
const Scalar<DataType>& lapse);

Copy link
Member

Choose a reason for hiding this comment

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

Factor this function into its own commit and add a test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -70,23 +99,78 @@ void stress_trace(gsl::not_null<Scalar<DataType>*> result,
(square(get(lorentz_factor)) * get(spatial_velocity_squared) + 1.);
}

template <typename DataType>
void stress_energy_tensor(
gsl::not_null<tnsr::AA<DataType, 3>*> result,
Copy link
Member

Choose a reason for hiding this comment

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

const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

get(pressure) + square(get(comoving_magnetic_field_magnitude));

const auto p_star =
get(pressure) + square(get(comoving_magnetic_field_magnitude)) / 2.;
Copy link
Member

Choose a reason for hiding this comment

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

don't divide by two, multiply by 0.5 on the left

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

const tnsr::I<DataType, 3>& magnetic_field,
const tnsr::ii<DataType, 3>& spatial_metric,
const tnsr::II<DataType, 3>& inverse_spatial_metric) {
const auto inverse_spacetime_metric =
Copy link
Member

Choose a reason for hiding this comment

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

I'm generally unhappy about the large number of allocations in this function. It would be good to switch to the not_null versions and preallocate in a Variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@jyoo1042 jyoo1042 force-pushed the Stress branch 2 times, most recently from 4046b96 to 175779d Compare February 6, 2025 17:16
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.

3 participants