-
Notifications
You must be signed in to change notification settings - Fork 841
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 driver/config restructure bug #751
Conversation
@pcarruscag I would not call that an avoidable bug considering the fact that the regression test passed. |
It passed because the config options were not updated and so no mesh deformation was specified for any of the FSI cases. |
If this has no influence on the test values then we should change these cases I would say. |
Let’s try to keep it positive here.. @talbring has an interesting point that the tests should show some sensitivity to major changes like the grid deformation not being specified. Can they be made more sensitive to this? |
I agree, but some do pick up those changes, just not all of them. Anyway, you'll notice I put an assert for the condition that was causing the problem instead of an "mpi error". That is inline with the philosophy that errors/exceptions are to deal with user errors and asserts to help debugging (they are ignored with -DNDEBUG). I can off-course change it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just reproduced this error and indeed all static FSI cases fail, so let's merge the bugfix in.
Idea is, replacing FLUID_STRUCTURE_STATIC and FLUID_STRUCTURE with the new mesh solver, so these options should change again soon (and I'll make sure they get properly documented).
Still, I think we should add another steady-state FSI test case, to catch this kind of bugs in the regression tests. Let me do that in the coming PR.
Proposed Changes
@rsanfer , @talbring please keep the testcases up to date with the new options you are introducing, avoidable bugs are starting to pass...
PR Checklist