Skip to content
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

Change handling of boundary conditions #336

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

A-CGray
Copy link
Contributor

@A-CGray A-CGray commented Nov 1, 2024

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

  • TACS doesn't doesn't have the ability to compute a MatVec product with a transposed TACSSchurMat with boundary conditions correctly applied
    • Therefore, the addTransposeVectorProduct of the static problem uses the non-transposed matrix, which produces a slightly incorrect result
    • This should result in the MPhys partial derivatives tests failing, but we get around this in a few ways:
      • We use quite a slack tolerance on the mphys partial derivatives tests (rtol=1e-1)
      • We overwrite the constrained DOF with the correct Dirichlet values inside the static problem's setVariables method. This makes the jacobian computed by OpenMDAO's finite differencing slightly more like the one computed using addTransposeVectorProduct
      • Because OpenMDAO tests the norm of the error over the whole jacobian, the test can pass even though some entries related to the BCs are completely wrong

addSVSens

  • The addSVSens method of the TACSAssembler 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 values
    • This should mean that the MPhys tests of the function partial derivatives w.r.t states fail.
    • However, overwriting the constrained DOF with the correct Dirichlet values inside the static problem's setVariables method means the finite-difference partials with respect the constrained DOF appear to be zero, matching the result of addSVSens and thus passing the check_partials tests.

Proposed changes

  • Remove call tosetBCs in setVariables so that we're not overwriting the states OpenMDAO thinks it is setting
  • Do not zero out boundary condition terms when calling addSVSens in static problem
  • Add an applyBCs argument (that defaults to true) to the TACSAssembler functions that involve applying BCs:
    • assembleRes
    • assembleJacobian
    • assembleMatType
    • assembleMatCombo
    • addSVSens
    • evalMatSVSensInnerProduct
    • addJacobianVecProduct
    • addMatrixFreeVecProduct
  • Fix addTransposeJacVecProduct in static problem by using the TACSAssembler addJacVecProduct method with the transpose option enabled and the boundary conditions not applied so that it gives the correct result
  • Tighten tolerances in MPhys derivative tests since our partial derivatives should now be correct
  • Implement a scaling factor on the Dirichlet BC residual analagous to the load factor we have for scaling external forces, so that load control and displacement control can be performed using a single scaling factor.

For 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

  • How does this BC handling apply to transient and buckling problems?

@A-CGray
Copy link
Contributor Author

A-CGray commented Nov 1, 2024

With the exception of test_thermoelast_linquad_element_2d.py, I think all the remaining test failures are from transient problems, I think those failures are due to addSVSens no longer applying BCs. Will need some input from somebody familiar with the unsteady adjoint implementation to fix that.

@timryanb
Copy link
Collaborator

Small thing, but can we update the docstrings in tacs.pyx to include the new applyBCs flag?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants