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

Sparse partials for ExecComps #63

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

kanekosh
Copy link
Contributor

Purpose

I added has_diag_partials=True for the ExecComps with vector inputs and outputs. This tells OpenMDAO that those components have sparse diagonal partials. This should (slightly) reduce the runtime by eliminating the OM's automatic Jacobian coloring computations.

Type of change

  • Refactoring (no functional changes, no API changes)

Testing

Checklist

Put an x in the boxes that apply.

  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6fc0a1c) 79.95% compared to head (d4e2a58) 79.95%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #63   +/-   ##
=======================================
  Coverage   79.95%   79.95%           
=======================================
  Files          85       85           
  Lines        9300     9300           
=======================================
  Hits         7436     7436           
  Misses       1864     1864           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kanekosh kanekosh marked this pull request as ready for review February 21, 2024 14:29
@kanekosh kanekosh requested a review from a team as a code owner February 21, 2024 14:29
@eytanadler
Copy link
Collaborator

eytanadler commented Feb 21, 2024

When has_diag_partials is set to True, does OpenMDAO perturb all elements in a given vectorized input simultaneously when computing the complex step derivatives? Many of these components can be replaced with OpenConcept's ElementMultiplyDivideComp, which might be even better. You have a better sense than I do of the times for coloring vs. actually computing derivatives.

@kanekosh
Copy link
Contributor Author

When has_diag_partials is set to True, does OpenMDAO perturb all elements in a given vectorized input simultaneously when computing the complex step derivatives?

Yes, that's what OM does (reference). ElementMultiplyDivideComp would be slightly better but I guess the speedup is minimal because that's just one additional compute evaluation with complex numbers in ExecComp.

Copy link
Collaborator

@eytanadler eytanadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, this looks good then. The Python 3.8 flake8 test is no longer run because of mdolab/.github#64, which was merged yesterday. I've updated the required tests in the OpenConcept repo to reflect this. I'll try rerunning the tests and see if they update, but if not I guess I can just override the tests and merge it.

@eytanadler eytanadler merged commit 773de3a into mdolab:main Feb 21, 2024
13 of 14 checks passed
@kanekosh kanekosh deleted the sparse_partials branch May 31, 2024 22:53
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