Improve Performance for OptimizationBBO #600
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
This is so nothing breaks, if callback is called if its nothing
Not entirely sure what was going on, but we started boxing variables again I think (see SciML#519)
Currently I get 4609144 bytes allocated, whilst with `callback = (args...) -> false` I get 69929923 which should be a lower bound if the fixes in this pr were undone.
Co-authored-by: Christopher Rackauckas <[email protected]>
ChrisRackauckas
approved these changes
Oct 1, 2023
Codecov Report
@@ Coverage Diff @@
## master #600 +/- ##
=========================================
+ Coverage 8.89% 9.03% +0.14%
=========================================
Files 40 40
Lines 2688 2690 +2
=========================================
+ Hits 239 243 +4
+ Misses 2449 2447 -2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
This fixes a performance regression due to 92974b8. The regression is because the defaults for
callbackanddatawere changed without changing the code that branched on whether they were set to their defaults.I removed
fieldnamesas that seems to not run at compile time (it was taking about 50% of the time for my use case due to having to iterate over it), probably because its effects are(!c,!e,!n,!t,!s,!m,+i)′. I believe you could fix this by using generated functions but I don't know how to.Cthulhu complains that calling
getpropertywithu0andphas anAnyreturn type, though I don't see it in the profiler so maybe Cthulhu is wrong here? If this is an issue, I'm not sure how to fix it, perhaps this feature should be removed.Some tests should be added to prevent this regression in the future, I've added a test to check for how many bytes are allocated.