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

Part 1 of setup refactor. #312

Merged
merged 13 commits into from Aug 3, 2017
Merged

Part 1 of setup refactor. #312

merged 13 commits into from Aug 3, 2017

Conversation

Kenneth-T-Moore
Copy link
Member

  1. New group method configure where users can make changes to their subsystems. These are recursed in reverse.
  2. Moved things around so that all groups and components have their setup called under setup_procs.

This method may optionally be overidden by your Group's method.

You may only use this method to change settings on your children subsystems. This includes
setting solvers in cases where you want to override the defaults.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should include some detail about timing here.

"You can assume that the full hierarchy below your level has been instantiated and has already called its own configure methods."

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@JustinSGray JustinSGray left a comment

Choose a reason for hiding this comment

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

want a few minor changes to the test asserts


top['sub.comp.a'] = 1.5576015661428098
top.run_model()
assert_rel_error(self, top['sub.comp.x'], -0.5)
Copy link
Member

Choose a reason for hiding this comment

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

should you explicitly test for the solve class here as well. Testing for convergence seems a round-about-way to know this worked here.

Copy link
Member

Choose a reason for hiding this comment

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

also, the test could be a bit faster if we didn't actually have to run the solver. Since we're not testing solve behavior here, its not necessary and would even, I think make a better more unit-like test if we didn't run the solver

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@naylor-b naylor-b merged commit 6beac7a into OpenMDAO:master Aug 3, 2017
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.

None yet

3 participants