Update flow dependencies #588
Merged
Conversation
This file contains 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
- Make writer flows depend on the ip flows - Add reapply policy to teh ModelGraph.apply_flows() - Move the stamp to the optimizer
thesps
reviewed
Jul 5, 2022
thesps
reviewed
Jul 5, 2022
| length = 8 | ||
| return ''.join(choice(hexdigits) for m in range(length)) | ||
|
|
||
| model.config.config['Stamp'] = _make_stamp() |
There was a problem hiding this comment.
I know this isn't the point of this PR, but would it be safer to make the stamp a (hidden?) attribute of the model rather than a config entry?
There was a problem hiding this comment.
Yes! I was thinking that since layers have attributes, so should the model. Then all the config stuff that must be refactored can be implemented this way. But it's for the next PR.
|
Perhaps some tests with dummy models and dummy flows, with asserts on the |
|
I can add the test, but it would be pretty naive. It will test only the |
thesps
approved these changes
Jul 7, 2022
calad0i
pushed a commit
to calad0i/hls4ml
that referenced
this pull request
Jul 1, 2023
* Shuffle the flows a bit - Make writer flows depend on the ip flows - Add reapply policy to teh ModelGraph.apply_flows() - Move the stamp to the optimizer * Remove the printout * Add a test for flows Co-authored-by: Sioni Summers <sioni.summers10@imperial.ac.uk>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR shuffles the order of flows a bit to help resolve the issues observed in #509 and elsewhere. Essentially it makes writer flows depend on the ip flows and expands functionality of
apply_flows()to control how the flows are reapplied if they were already applied before. Another effect of the change is that the flows aren't applied multiple times, an issue that many people have observed.Type of change
Tests
No extra tests are needed. To reproduce the old behavior, just convert any model and call
compile()andbuild()and see how many times the same optimizers will be applied (inmodel._applied_flows).Checklist