Fix client metadata accumulating across repeated setups - #82
Merged
Merged
Conversation
The dedup guard in DisciplineClient.get_partials_definitions compared a str against a list of PartialsMetaData messages, so it never fired and every streamed message was appended unconditionally. get_variable_definitions had the same append-rather-than-replace shape with no guard at all. Both methods now clear their metadata lists before repopulating them, which mirrors the _clear_data() the server performs at the start of each Setup. A client that is set up more than once -- reused across jobs, for instance -- now replaces its metadata instead of accumulating duplicate Jacobian preallocations in _recover_partials and duplicate declare_partials calls in the OpenMDAO binding. Fixes #78
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #78.
Problem
The dedup guard in
DisciplineClient.get_partials_definitionscompared astragainst a list ofPartialsMetaDatamessages, somessage.name not in self._partials_metawas always true and every streamed message was appended unconditionally.get_variable_definitionshad the same append-rather-than-replace shape a few lines above, with no guard at all.Latent today — nothing in the repo sets a client up twice — but reachable as soon as a client is reused across jobs. A second setup would leave
_recover_partialspreallocating the same Jacobian block repeatedly anddeclare_partialsin the OpenMDAO binding declaring each pair more than once.Fix
Both methods now clear their metadata lists before repopulating them, which mirrors the
_clear_data()the server performs at the start of eachSetup. The broken guard is dropped rather than repaired — with the list cleared each call there is nothing left to dedup against.This takes the second option the issue proposed, and extends it to
get_variable_definitionsso the two getters behave the same way.Tests
Two regression tests in
tests/test_discipline_client.pycall each getter twice against the same mocked stream and assert the counts do not double; both fail on the currentdevelop(4 partials instead of 2). Full suite passes: 311 passed, 15 subtests passed.