Closed bindi-nagda closed 6 months ago
The tests that were added check that the solver outputs the expected error_norms for apply
, multigrid
, time_stepping
, time_stepping_thn
. For target frm
, the tests simply check that both the newtonian and complex fluid simulation can run for a few steps. It doesn't look at the output to check for number of iterations to converge or relative residuals. If we need to check for other items for the frm solve, I can certainly add them.
@abarret We need to add the tests for convec_op and physical_bcs. This PR can be merged into main in the meantime. I would be happy to add the tests for physical_bc and convec_op either by pushing directly to their branch or making a new PR later, whichever is more appropriate.
number of iterations to converge or relative residuals
These things are sensitive to finite precision order of operations and are not reproducible.
I would be happy to add the tests for physical_bc and convec_op either by pushing directly to their branch or making a new PR later, whichever is more appropriate.
We can make a new PR for those. I would rather get those merged in sooner.
Going to update this PR to resolve merge conflicts, and add unit tests for convection operator and physical boundary conditions.
Going to update this PR to resolve merge conflicts, and add unit tests for convection operator and physical boundary conditions.
Can you just resolve the merge conflicts, and add new unit tests to a separate PR? I would like to go ahead and get this merged.
@abarret After rebasing with main, I ran into some issues with unit tests failing for targets apply
and multigrid
. Using gdb, I found seg faults occurring when calling the method CacheBcCoefData
in VCTwoFluidStaggeredStokesOperator
. I fixed these issues by creating boundary condition objects and passing them to stokes_op. This seemed to work with all unit tests now passing, but perhaps there's a better way to do this?
Otherwise this is ready to merge..
Yeah. I think the better approach is to make the physical bc helper optional in the operator. I'll push that change in the morning and merge.
This adds tests for targets
apply
,multigrid
,time_stepping
,time_stepping_thn
andfrm
.