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 test for ShellMatrix #3606

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open

Conversation

BalticPinguin
Copy link
Member

Following @roystgnr s comment 'we don't have test coverage', here is some.
It doesn't cover too much; but maybe it at least is a
And it would have covered the issue #3602 .

@roystgnr
Copy link
Member

I didn't think we'd turned on whitespace requirements for libMesh, even on just "spaces vs tabs"... The fact that the error message is giving MOOSE formatting rather than libMesh formatting advice isn't a good sign.

You might try the contrib/bin/reindent.sh script, or if you don't have emacs on your system let me know and I can push a commit with whitespace fixes.

@BalticPinguin BalticPinguin force-pushed the master branch 2 times, most recently from 9bfe917 to 1c23015 Compare July 31, 2023 16:55
@BalticPinguin
Copy link
Member Author

sorry for the repeated pushing; I got messed up with rebasing these few lines...

@BalticPinguin
Copy link
Member Author

hm; you run unit-tests with ./unit_tests-opt -pc_type asm -pc_asm_overlap 4 ..... ; but this is not supported by shell-matrices.
I can define a SolverConfiguration-object, where I set PCType to NONE, which overrides the command line option.
Do you consider this cheating on the test (since I simply bypass the specified options)?

@roystgnr
Copy link
Member

Make sure it doesn't fail (even if only because it's #ifdefed out) on non-PETSc builds; if so, that'd be a very good solution. IIRC we use those options on examples because they make some of the trickier Jacobians more robust to partitioning, and we only even use them on unit tests by accident, because the same make check invokes both.

@BalticPinguin
Copy link
Member Author

What is the reason that the civet-tests don't run? Is there something I can do about?
Best regards

@roystgnr
Copy link
Member

roystgnr commented Aug 9, 2023

We were having Civet issues, so perhaps the trigger never triggered? Try rewording a commit message or rebasing on master or something and then force push to see if it'll trigger a run now?

@moosebuild
Copy link

Job Test MOOSE mac on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Test with threads on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Test debug on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Test clang on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Test 32bit on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Distributed make check sweep (odd) on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@moosebuild
Copy link

Job Distributed make check sweep (even) on 6f6a14f : invalidated by @jwpeterson

Retry after CI issues resolved

@jwpeterson
Copy link
Member

@BalticPinguin there seems to be a real issue with your PR which is causing the unit tests (presumably due to your new test) to time out, which subsequently causes them to be automatically cancelled. For example, in the "Test with threads" test, you can see that the unit tests are running for an hour, which is not normal:

CIVET: Cancelling job due to step taking longer than the max 3600 seconds

I haven't looked into what might be causing this yet, but it seems at least possible that the issue is related to threads or to the new unit test running in parallel, so I'd suggest to start your investigation there.

@BalticPinguin
Copy link
Member Author

Thank you @jwpeterson for the hint: I haven't noticed the 'threads'-part.
When adding this, I can reproduce the behaviour; I'll look into this soon.

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.

4 participants