-
Notifications
You must be signed in to change notification settings - Fork 47
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
Fluids - Compute component-wise error for NS solver #1109
Conversation
I think this branch was working for our purposes in the course project but is broken for the Advection2D and Euler solvers. A quick fix would be to define a |
6407e75
to
3d369af
Compare
Hmm, I think |
Oh, true! I was talking about Newtonian since that is the only solver that uses |
Sounds good to me. Newtonian is what matters for us (and at some point in the next few months, I think we'll add RANS). |
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.
Looks good. Perhaps a topic for another day, but I wonder if the velocity error norm should be defined using the pointwise relative error
Primitive variables:
Component 0: 6.06303e-05 (24.4948, 404002.)
Component 1: 0.00177814 (0.276762, 155.647)
Component 2: 5.3239 (0.256163, 0.0481156)
Component 3: -nan. (0., 0.)
Component 4: 0.00184015 (2.93409, 1594.48)
We could do that in postprocessing by comparing absolute error as a vector to the true solution norm as a vector. (What's happening here is that velocity is very accurate in total, but not in exactly the same direction as the exact solution, thus we get a small y component error that is "big" compared to the exact y component that is almost perfectly aligned with the axis.)
ba1253a
to
ecf2834
Compare
@jedbrown , does this look good to you now? |
0694436
to
fbef8a9
Compare
fbef8a9
to
447e215
Compare
@jedbrown , can we merge this branch if there are no other concerns? |
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.
Looks pretty close. Could you test or at least show sample output, perhaps using the Euler vortex?
0ef6d47
to
f9ca350
Compare
Here is the output with the traveling vortex problem:
|
f9ca350
to
f770efc
Compare
Why does it always say |
It's because traveling vortex doesn't support primitive variables yet. |
I remember we didn't see convergence with |
I took the arguments from the regression tests in |
Do you mean a convergence plot? |
Well, something that runs this code and checks that the errors are of expected size. (What if a bug caused the reported error to be zero always or the error became huge?) Ideally we would make a convergence plot for the isentropic vortex or channel flow, but I can handle that not being ready. |
cebaf41
to
860a18d
Compare
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.
Thanks for getting this done!
Cool. Squash or squash-merge? |
Thanks, @jrwrigh for bearing with me. A lot has changed since the last time I contributed to this mini-appand. There are still a couple of remaining tasks before merging:
|
squash-merge is fine after I am done with the remaining tasks. |
a9175ef
to
b04efdf
Compare
I am seeing a weird issue. The errors are not correct with rank 1.
(same issue with advection2d and blasius) |
Testing with different arguments/problems, I see that the results could be different for n>1 as well. |
41a9f52
to
09cea8b
Compare
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.
Cool. Can we test isentropic vortex now or does it need some conversion? Do we have any problems in which we can show convergence under refinement?
Unfortunately, isentropic vortex doesn't have support for primitive variables. If we want to test the full functionality, we should either make the channel problem compute the errors (I tried but it was more complicated than switching on a flag) or, refactor the euler solver to use newtonian (I think this is the simplest way to make traveling vortex functional for our desired convergence study). Or we can just show the convergence with isentropic vortex using conservative variables (without the conversion). This is ready! However, I am still concerned about this issue. Could you take a look at the functions I defined/modified in |
c5760fc
to
de198a6
Compare
error to the target state variable
0ff14ef
to
7dd523d
Compare
@jedbrown , I think I fixed the issues you raised the other day but I think the problem I am seeing with the parallel run could be irrelevant to this PR. Serial
Parallel:
Basically, the solver is diverging with parallel runs while there is no problem with the serial run. Is this normal? Oh, I shouldn't have squashed the commits, I though it could help. You can see your suggestions here. |
Why is the nonlinear solve diverging in parallel? Is the linear solver failing? Is this solely a difference of the preconditioner getting weaker in parallel for the steady problem?
|
The failure is very random and it happens in the linear solver. I ran some tests on Noether with
(I don't see any preconditioning being set in the yaml file.) |
You can use |
Oh, yes. Those runs and the following are all on
|
Closing in anticipation of moving Honee to a new repo this seems like a good branch to recreate on the other side of the move! We should make an issue when we stand up the new repo @jrwrigh |
This is part of a course project in collaboration with @jnclement and @amandashack . It adds support for computing the error for each component and converts the errors to the other set of variables (primitive <-> conservative). It currently breaks for the other solvers though (advection and euler).