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

Fix compiling deal.II with PETSc and complex scalar type #15355

Merged
merged 2 commits into from Jun 14, 2023

Conversation

masterleinad
Copy link
Member

There are a lot of failing tests but this pull request at least fixes compiling the library.

@masterleinad
Copy link
Member Author

/rebuild

Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Hm, so pretty much all the things that are in need of changing are in that one place we've been talking about. It probably deserves a big fat TODO.

Comment on lines -212 to +214
solution(idx) = copy(idx) + constant_adjustment;
solution(idx) =
typename VectorType::value_type(copy(idx)) +
constant_adjustment;
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand this change. copy is of type VectorType. How could one of its elements be of something other than VectorType::value_type?

Copy link
Member

Choose a reason for hiding this comment

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

Or, put differently: What is the error message you see here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The type of copy(idx) is internal::VectorReference and overload resolution for operator+(internal::VectorReference, const complex<double>) fails (but VectorReference has a conversion operator to PetscScalar`):

/tmp/dealii/include/deal.II/numerics/vector_tools_mean_value.templates.h:212:49: error: invalid operands to binary expression ('internal::VectorReference' and 'const typename Vector::value_type' (aka 'const complex<double>'))
                      solution(idx) = copy(idx) + constant_adjustment;
                                      ~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
/tmp/dealii/build/source/numerics/vector_tools_mean_value.inst:5460:2: note: in instantiation of function template specialization 'dealii::VectorTools::add_constant<dealii::PETScWrappers::MPI::Vector, 1, 2>' requested here
 add_constant<>( PETScWrappers::MPI::Vector  & solution,
 ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/complex:547:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('dealii::PETScWrappers::internal::VectorReference' vs. 'double')
operator+(const _Tp& __x, const complex<_Tp>& __y)
^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/__iterator/move_iterator.h:172:1: note: candidate template ignored: could not match 'move_iterator' against 'complex'
operator+(typename move_iterator<_Iter>::difference_type __n, const move_iterator<_Iter>& __x)
[...]

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. That makes sense, even though it's awkward.

@@ -198,7 +198,7 @@ namespace PETScWrappers
OutType
operator*() const
{
return static_cast<OutType>(*m_iterator);
return static_cast<OutType>(std::real(*m_iterator));
Copy link
Member

Choose a reason for hiding this comment

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

This just puts the whole idea of the ConvertingIterator to lies. At least it deserves a TODO :-)

&array[n_elements_stored_locally]))
&array[n_elements_stored_locally],
[](PetscScalar left, PetscScalar right) {
return std::real(left) < std::real(right);
Copy link
Member

Choose a reason for hiding this comment

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

It may be clearest if you wrote this as follows:

Suggested change
return std::real(left) < std::real(right);
return static_cast<PetscInt>(std::real(left)) < static_cast<PetscInt>(std::real(right));

Comment on lines 297 to 307
std::vector<PetscInt> sorted_indices(
&array[end_index - ghost_start_index],
&array[n_elements_stored_locally]);
std::vector<PetscInt> sorted_indices;
sorted_indices.reserve(n_elements_stored_locally -
(end_index - ghost_start_index));
for (PetscInt i = end_index - ghost_start_index;
i < n_elements_stored_locally;
++i)
sorted_indices.push_back(
static_cast<types::global_dof_index>(std::real(array[i])));
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't you do this with the ConvertingIterator as well? Just write it like this:

std::vector<PetscInt> sorted_indices(
              ConvertingIterator<...>(&array[end_index - ghost_start_index]),
              ConvertingIterator<...>(&array[n_elements_stored_locally]));

@bangerth bangerth merged commit f92cc40 into dealii:master Jun 14, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants