Run MFNavierStokesPreconditionGMG in float #1319
Conversation
| using OperatorType = NavierStokesOperatorBase<dim, double>; | ||
| using SmootherPreconditionerType = PreconditionBase<VectorType>; | ||
| using Number = double; | ||
| using MGNumber = float; |
There was a problem hiding this comment.
The place to switch between float and double.
There was a problem hiding this comment.
We might need a compiler flag to change this accordingly?
There was a problem hiding this comment.
I am not sure how a compiler flag would help. Switching between float and double directly in the code could seems to be easy enough.
|
@peterrum I think the test that was breaking was breaking because the coarse grid solver was not a direct solver (for no reason at all). @lpsaavedra I think we should change the default behaviour of the coarse grid solver so that default is direct. In general, except in very particular cases, we should always use the direct solver I think. |
|
I have been checking the reason why this test is not working and I realized that the same happens for the master version in both release and debug mode. It seems that the direct solver (as coarse grid solver) does not work when specifying a number of minimum cells for the levels. If we set this to -1, it works just fine. I can modify this accordingly... however, I need a long term solution for this, so I will keep looking. @blaisb I agree with you that we should in fact change this to default and use it for all tests. I will try this change now and see if I encounter similar problems. |
|
In any case, without specifying the minimum number of cells (i.e., using the coarsest level for the coarse solver), we do get the exact same values for the enstrophy for 1 second using both float and double. I guess this is good, but I think I will have to run a complete TGV problem to see what is the effect of changing this to float. I will keep you posted. |
Perfect. Maybe that's because the direct solver would need the set pressure dirichlet to true to fix the pressure reference? In the end this is a periodic problem so the pressure reference can be illposed. |
|
Yes, you are right, that seems to fix it as well. I will update the test in this branch. Do we want to keep the minimum number of cells or do we fix the pressure constant? Do you have any preference? |
I don't have a preference, but I feel like fixing the pressure reference is a more robust solution. i'll trust your judgment here :) |
|
@lpsaavedra Thank you fixing the issue with the test. The issue seems to be unrelated to the current PR. What about extracting those changes in a seperate PR, which we first merge? |
|
I have fixed the test in a different PR (#1321). |
|
Perfect, we can rebase this PR. |
|
I have added a compiler flag for this feature |
No description provided.