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

POEM 071: Change ExecComp to use declare_coloring #148

Merged
merged 5 commits into from
Dec 19, 2022
Merged

Conversation

robfalck
Copy link
Contributor

@robfalck robfalck commented Sep 2, 2022

No description provided.

@robfalck robfalck changed the title POEM_071 - Change ExecComp to use declare_coloring POEM_071: Change ExecComp to use declare_coloring Sep 4, 2022
@robfalck robfalck changed the title POEM_071: Change ExecComp to use declare_coloring POEM 071: Change ExecComp to use declare_coloring Sep 4, 2022
POEM_071.md Outdated
The option `has_diag_partials` on ExecComp was added prior to the development of partial derivative coloring to provide a common sparsity pattern for vectorized components.

With the advent of partial coloring, `has_diag_partials` is no longer needed.
Users should be able to expect that `ExecComp` will determine the sparsity pattern of calualtions involved.
Copy link
Member

Choose a reason for hiding this comment

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

spelling: calualtions

POEM_071.md Outdated

## Changes

1. `has_diag_partials` will be marked as deprecated but still be allowed as an option that does nothing.
Copy link
Member

Choose a reason for hiding this comment

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

We ended up leaving the old internal CS stuff intact because that allows us to use CS local to the ExecComp without requiring the full model vectors to be allocated as complex, and has_diag_partials offers a performance improvement over computing the coloring so I left it in (with no deprecation).

POEM_071.md Outdated
## Changes

1. `has_diag_partials` will be marked as deprecated but still be allowed as an option that does nothing.
2. `declare_coloring_kwargs` will allow the user to specify any arguments to be passed to [System.declare_coloring](https://openmdao.org/newdocs/versions/latest/features/experimental/approx_coloring.html#dynamic-coloring))
Copy link
Member

Choose a reason for hiding this comment

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

declare_coloring_kwargs wasn't used. The thought process was that if a user has specific args they need to pass (usually not the case), then they can just call declare_coloring and pass them normally. Otherwise, if they don't call declare_coloring explicitly, they get the internal coloring method that only uses default args.

@robfalck robfalck merged commit 16c9cc1 into master Dec 19, 2022
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.

3 participants