Change handling of boundary conditions #336
Draft
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problems
In the static problem class, I noticed that we overwrite the constrained DOF with the correct Dirichlet values inside the static problem's
setVariables
method. This shouldn't be necessary as the boundary conditions are already accounted for in the residual and jacobian such that the solution of the static problem automatically satisfies the BCs. Additionally, overwriting these BC values means our MPhys wrapper is not allowing OpenMDAO to set the states it thinks it is (which has the potential to cause bugs depending on how you're using TACS through MPhys). It also means we cannot perform displacement control for nonlinear solutions, since the dirichlet BCs cannot be applied incrementally. Digging a bit deeper, I found that the reason we do this overwriting is primarily to make OpenMDAO's finite difference partial derivative checks pass for some partial derivatives terms that are not quite computed correctly by TACS as stands:addTransposeVectorProduct
TACSSchurMat
with boundary conditions correctly appliedaddTransposeVectorProduct
of the static problem uses the non-transposed matrix, which produces a slightly incorrect resultrtol=1e-1
)setVariables
method. This makes the jacobian computed by OpenMDAO's finite differencing slightly more like the one computed usingaddTransposeVectorProduct
addSVSens
addSVSens
method of theTACSAssembler
zeroes out the derivatives of function w.r.t DOFs that are subject to BCs, this is incorrect because the values of those DOFs still affect function valuessetVariables
method means the finite-difference partials with respect the constrained DOF appear to be zero, matching the result ofaddSVSens
and thus passing thecheck_partials
tests.Proposed changes
setBCs
insetVariables
so that we're not overwriting the states OpenMDAO thinks it is settingaddSVSens
in static problemapplyBCs
argument (that defaults totrue
) to theTACSAssembler
functions that involve applying BCs:assembleRes
assembleJacobian
assembleMatType
assembleMatCombo
addSVSens
evalMatSVSensInnerProduct
addJacobianVecProduct
addMatrixFreeVecProduct
addTransposeJacVecProduct
in static problem by using theTACSAssembler
addJacVecProduct
method with the transpose option enabled and the boundary conditions not applied so that it gives the correct resultFor a more thorough description of the math and why I think this way of handling the BCs is more correct, see TACS Static Problem Boundary Condition Handling.pdf
Open questions