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 AffineConstraints::is_closed() #13505

Merged
merged 1 commit into from Mar 8, 2022
Merged

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Mar 7, 2022

No description provided.

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.

OK with the requested changes.

@@ -770,6 +770,22 @@ class AffineConstraints : public Subscriptor
void
close();

/**
* Check if the function is_closed() was called or there are no
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Check if the function is_closed() was called or there are no
* Check if the function close() was called or there are no

is_closed() const;

/**
* Check if the function is_closed() was called or there are no
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Check if the function is_closed() was called or there are no
* Check if the function close() was called or there are no

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this does not make any sense ;)

* AffineConstraints was created for the DG case.
*/
bool
is_closed(const MPI_Comm &comm) const;
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it would make sense to select a more descriptive name here?

Suggested change
is_closed(const MPI_Comm &comm) const;
is_closed_on_all_processes(const MPI_Comm &comm) const;

(Given that the AffineConstraints class does not have the notion of the parallel distribution built-in.)

Copy link
Member Author

Choose a reason for hiding this comment

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

To be consistent would be is_closed_in_parallel (similar to is_consistent_in_parallel). The reason why I opted against this is that normally one calls or not close() on all processes (the only reason for having the reduction inisde to be able to decide if the index set is empty - since the constructor sets sorted automatically to false). If you think it is more clear, I can rename the function to is_closed_in_parallel.

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

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

Apart from my question, this looks great to me.

@kronbichler
Copy link
Member

/rebuild

@peterrum peterrum merged commit a5d36df into dealii:master Mar 8, 2022
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

3 participants