Skip to content

Change handling of boundary conditions#336

Open
A-CGray wants to merge 28 commits intosmdogroup:masterfrom
A-CGray:BC-Changes
Open

Change handling of boundary conditions#336
A-CGray wants to merge 28 commits intosmdogroup:masterfrom
A-CGray:BC-Changes

Conversation

@A-CGray
Copy link
Contributor

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

Motivation

In the static problem class, we overwrite the constrained DOF with the correct Dirichlet values inside the 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. 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.

addSVSens

The addSVSens method of the TACSAssembler zeroes out the derivatives of functions 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 to the constrained DOF appear to be zero, matching the result of addSVSens and thus passing the check_partials tests

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

Changes

New applyBCs flag on assembly methods

I added an applyBCs argument, which controls whether boundary conditions are applied to the resulting vector/matrix, to the following TACSAssembler methods, along with their corresponding Python bindings:

  • assembleRes
  • assembleJacobian
  • assembleMatType
  • assembleMatCombo
  • evalMatSVSensInnerProduct
  • addJacobianVecProduct
  • addMatrixFreeVecProduct

The argument defaults to true so that all the functions are backwards compatible.

Remove BC application from addSVSens

Rather than adding an applyBCs argument to addSVSens, I just removed the applyBCs call inside addSVSens entirely.

Unsteady adjoint fixes

Fixed the adjoint computation in the time integrators to correctly apply BCs after addSVSens (since addSVSens no longer applies them internally):

  • Added explicit applyBCs calls in TACSBDFIntegrator::initAdjoint
  • Added explicit applyBCs calls in TACSDIRKIntegrator::iterateAdjoint

Dirichlet BC scaling factor (lambda)

A lambda scaling parameter has been added to setBCs and applyBCs on TACSVec/TACSBVec (and TACSAssembler::setBCs). This scales the prescribed Dirichlet BC values, analogous to the load scaling factor we already have for external forces. This enables load control and displacement control to be performed using a single scaling factor.

Static problem fixes

  • Removed the call to setBCs in setVariables so that we're not overwriting the states OpenMDAO thinks it is setting
  • Removed a redundant applyBCs call on the adjoint solution vector after the linear solve

Test updates

  • Tightened MPhys derivative test tolerances (complex-step rtol: 1e-71e-10, finite-difference rtol: 1e-21e-4)
  • OpenMDAO base test: residual partials are no longer excluded from complex-step checks (still excluded for finite difference where they can't be computed accurately)
  • Static analysis base test: added explicit applyBCs call to adjoint RHS before solving, matching the corrected workflow
  • Fixed dvNum bug in PCM element test (was hardcoded to 1 instead of using dv_num)

Open questions

  • How does this BC handling apply to 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?

@A-CGray
Copy link
Contributor Author

A-CGray commented Feb 27, 2025

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.

The error in test_thermoelast_linquad_element_2d was because the stiffness matrix is non-symmetric in thermoelastic problems. Unlike the static problem, static_analysis_base_test actually assembles the transposed matrix for its adjoint solves, which I had undone but now reverted. This transposed matrix is technically not the correct matrix for the adjoint solve because BCs are still applied by zeroing rows, not columns, but it seems to be good enough to pass the tests. Presumably though this means that the current static problem implementation doesn't compute accurate derivatives for thermoelastic problems (we don't test this).

@A-CGray
Copy link
Contributor Author

A-CGray commented Feb 27, 2025

Not sure what the issue with the unsteady adjoint is now, calling applyBCs on the output of addSVSens should give exactly the same result as calling it inside addSVSens but one of the tests is still failing

@timryanb
Copy link
Collaborator

Not sure what the issue with the unsteady adjoint is now, calling applyBCs on the output of addSVSens should give exactly the same result as calling it inside addSVSens but one of the tests is still failing

Should be addressed now

@A-CGray A-CGray marked this pull request as ready for review April 4, 2025 23:54
@A-CGray A-CGray marked this pull request as draft July 14, 2025 19:40
@A-CGray A-CGray mentioned this pull request Jan 23, 2026
@A-CGray A-CGray marked this pull request as ready for review February 6, 2026 01:45
@A-CGray A-CGray removed the request for review from sean-engelstad February 9, 2026 15:17
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