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

Support complex numbers in PartialDerivatives #6107

Merged
merged 6 commits into from
Aug 23, 2024

Conversation

nilsvu
Copy link
Member

@nilsvu nilsvu commented Jun 22, 2024

Proposed changes

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

@nilsvu nilsvu marked this pull request as draft June 22, 2024 05:35
@nilsvu nilsvu changed the title Support complex numbers in a few more places Support complex numbers in PartialDerivatives Jun 22, 2024
@nilsvu nilsvu force-pushed the complex_numbers branch 7 times, most recently from 26aa312 to 37d9973 Compare June 25, 2024 22:47
@nilsvu nilsvu force-pushed the complex_numbers branch 9 times, most recently from 0e20438 to a4bbc8f Compare August 21, 2024 04:26
@nilsvu nilsvu marked this pull request as ready for review August 21, 2024 05:58
@nilsvu
Copy link
Member Author

nilsvu commented Aug 21, 2024

@nilsdeppe or @wthrowe would one of you take this one?

SPECTRE_TEST_CASE("Unit.Numerical.DiscontinuousGalerkin.ApplyMassMatrix",
"[NumericalAlgorithms][Unit]") {
template <typename DataType>
void test_apply_mass_matrix() {
Copy link
Member

Choose a reason for hiding this comment

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

Put this in the anonymous namespace.

0.0, // overwrite output with result
result, // result
matrix.rows()); // rows of result
add_to_result ? 1.0 : 0.0, // overwrite output with result
Copy link
Member

Choose a reason for hiding this comment

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

Update comment.

input, // input
matrix.columns(), // rows of input
std::complex{add_to_result ? 1.0 : 0.0,
0.0}, // overwrite output with result
Copy link
Member

Choose a reason for hiding this comment

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

Fix comment.

imaginary_part = std::complex<double>(0., static_cast<double>(i) + 3);
for (size_t d = 0; d < Dim; ++d) {
imaginary_part *=
std::complex<double>(0., static_cast<double>(d) + 2) *
Copy link
Member

Choose a reason for hiding this comment

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

Should this be real? Multiplying by an imaginary number makes imaginary_part real half the time. Same thing a couple more times below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed a fixup for this

wthrowe
wthrowe previously approved these changes Aug 21, 2024
Copy link
Member

@wthrowe wthrowe left a comment

Choose a reason for hiding this comment

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

Looks good. Squash.

@nilsdeppe I think you said you wanted to take a final look at this?

@nilsvu nilsvu force-pushed the complex_numbers branch 2 times, most recently from a2adda9 to 38c8f31 Compare August 22, 2024 04:52
@nilsvu
Copy link
Member Author

nilsvu commented Aug 22, 2024

Also added an extra commit at the end to add complex support to WeakDivergence.

Copy link
Member

@nilsdeppe nilsdeppe left a comment

Choose a reason for hiding this comment

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

@wthrowe can you look at the last commit too?

@nilsvu nilsvu requested a review from wthrowe August 23, 2024 00:43
Copy link
Member

@wthrowe wthrowe left a comment

Choose a reason for hiding this comment

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

I'll grumble about improper use of detail namespaces, but I'll allow it.

@wthrowe wthrowe merged commit 8203d93 into sxs-collaboration:develop Aug 23, 2024
22 checks passed
@nilsvu nilsvu deleted the complex_numbers branch August 23, 2024 19:02
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