Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds limited forward-mode support for the TACS MPhys wrapper (for state and RHS perturbations) and consolidates several vector copy patterns through shared utilities, along with a minor performance improvement in pyTACS BC application.
Changes:
- Introduces
copyToTACSVec/copyFromTACSVecutilities and refactors multiple problem/system methods to use them for consistent handling of TACS Vecs, NumPy arrays, and scalars. - Extends
StaticProblemwithaddJacVecProduct(unifying Jacobian/Jᵀ products) and a newsolveForwardmethod, and wires these intoTacsSolver.solve_linear/apply_linearto support forward-mode linear solves in MPhys. - Preallocates a temporary TACS Vec in
pyTACSfor applying boundary conditions to NumPy arrays and updates an integration test to exercise the new Newton + user-defined linear solver path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/integration_tests/test_mphys_struct_plate.py |
Tightens FD settings, switches to central differencing, and configures a Newton solver with LinearUserDefined to exercise the new forward-mode linear solve in the MPhys TACS wrapper. |
tacs/utilities.py |
Adds numpy import and defines copyToTACSVec / copyFromTACSVec static utilities on BaseUI to centralize copying between TACS Vecs and NumPy arrays/scalars. |
tacs/system.py |
Refactors setDesignVars and setNodes to use copyToTACSVec with unified error handling; note that the dict path in setNodes currently uses self.varName instead of self.coordName, so coordinate updates passed via dicts are ignored (commented as a bug). |
tacs/pytacs.py |
Allocates a reusable tempVec once and reuses it in applyBCsToVec / setBCsInVec when called with NumPy arrays, avoiding repeated TACS Vec allocations. |
tacs/problems/transient.py |
Uses copyToTACSVec in setInitConditions and copyFromTACSVec in getVariables, simplifying type handling and making the optional output arguments consistently respected. |
tacs/problems/static.py |
Replaces addTransposeJacVecProduct with a unified addJacVecProduct (with transpose flag), adds solveForward to solve J ψ = rhs, and refactors variable getters/setters and adjoint solve to use the new copy helpers; contains minor doc typos (commented) but otherwise aligns with the new forward-mode usage. |
tacs/problems/modal.py |
Updates getVariables to use copyFromTACSVec, simplifying returning eigenvectors into either TACS Vecs or NumPy arrays. |
tacs/problems/buckling.py |
Similarly updates getVariables to rely on copyFromTACSVec for buckling eigenvector extraction. |
tacs/mphys/solver.py |
Implements forward-mode solve_linear using StaticProblem.solveForward and forward-mode apply_linear using addJacVecProduct, including RHS and coordinate/DV handling; the forward RHS term currently uses d_residuals[self.states_name] instead of the RHS perturbation from d_inputs[self.rhs_name], so the forward Jacobian-vector product wrt RHS is incorrect (commented as a critical bug). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@timryanb I don't see any output from the test that's failing on the real Mac-OS build and I don't have a way to recreate that environment locally. Are you able to run the test locally and see what the issue is? My best guess is something PETSc related since the same test is passing in the Complex Mac-OS build which doesn't use the PETSc linear solver. |
Summary of changes:
Changes to MPhys solver class
Adds forward mode implementations for
solve_linearandapply_linear(for state and rhs perturbations) in theTacsSolverclass. This will not enable forward mode derivative calculation (because TACS doesn't have forward mode partials with respect to DVs and node coordinates) but should allow you to use TACS within coupled Newton solvers.I also added a few lines to
apply_linearto explicitly zero the output vectors before TACS adds to them. This didn't seem to be causing any issues before, but in my experience you can't rely on OpenMDAO having zeroed those vectors.I changed the setup of one of the MPhys integration tests to use OpenMDAO Newton and PETScKrylov nonlinear and linear solvers, with the TACS solver class's
solve_linearas the linear preconditioner. This will test both forward and reverse modes ofsolve_linearandapply_linear.New static problem methods
Adds static problem methods required for MPhys forward mode operations:
addTransposeJacVecProductwithaddJacVecProduct, which can compute normal and transpose Jac Vec productssolveForwardmethod, non-transposed equivalent tosolveAdjoint. Solves linear system with stiffness matrix and arbitrary RHSCode quality updates:
copyFromTACSVecandcopyToTACSVechelper functions to remove a bunch of duplicate codeapplyBCsToVecandapplyBCsInVecdon't allocate a TACS Vec every time they're called