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

Allow creating subvectors only supplying local indices #3857

Merged
merged 2 commits into from
May 24, 2024

Conversation

lindsayad
Copy link
Member

Also add some sanity checking about the parallel type of the subvector

Also add some sanity checking about the parallel type of the
subvector
@moosebuild
Copy link

moosebuild commented May 23, 2024

Job Coverage on 6bdf108 wanted to post the following:

Coverage

e7bc93 #3857 6bdf10
Total Total +/- New
Rate 62.70% 62.70% -0.01% 0.00%
Hits 69239 69239 - 0
Misses 41182 41192 +10 12

Diff coverage report

Full coverage report

Warnings

  • New new line coverage rate 0.00% is less than the suggested 90.0%

This comment will be updated on new commits.

Comment on lines -726 to +727
const std::vector<numeric_index_type> &) const
const std::vector<numeric_index_type> &,
bool = true) const
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some documentation for the new parameter? Since it doesn't even have a name here in the base class declaration, it's a bit mysterious as to what it might be for...

Copy link
Member Author

Choose a reason for hiding this comment

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

My very poor excuse for why I didn't do this initially is that I will have to comment out the parameter name to avoid unused warnings. Will a commented out parameter name still show up in the doxygen? Either way I can still articulate something in the doxygen wrt that parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

relevant issue: doxygen/doxygen#6926

Copy link
Member

@jwpeterson jwpeterson 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 to me, I'll merge later today unless @roystgnr has any objections or gets to it first.

@jwpeterson
Copy link
Member

Test MOOSE mac failure definitely seems unrelated:

Traceback (most recent call last):
  File "/opt/civet0/build/moose/python/MooseDocs/test/commands/test_check.py", line 33, in testCheck
    self.assertIn('MOOSEAPP REPORT(S):', out)
AssertionError: 'MOOSEAPP REPORT(S):' not found in 'None'

@jwpeterson jwpeterson merged commit 6cd7a31 into libMesh:devel May 24, 2024
20 of 21 checks passed
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