-
Notifications
You must be signed in to change notification settings - Fork 37
Remove unnecessary consistency checks for VarNamedVector #1092
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
Conversation
Benchmark Report for Commit 427581bComputer InformationBenchmark Results |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1092 +/- ##
==========================================
- Coverage 81.43% 81.17% -0.26%
==========================================
Files 40 40
Lines 3749 3793 +44
==========================================
+ Hits 3053 3079 +26
- Misses 696 714 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Some benchmarks of the Smorgasbord model of VNV vs Metadata. EDIT: The On main: On this PR: |
|
Test failures are the same as on main. |
Co-authored-by: Penelope Yong <[email protected]>
|
DynamicPPL.jl documentation for PR #1092 is available at: |
|
Fixed benchmarks: On main: On this PR: You may wonder why the whole scale of these numbers as jumped up, compared to the earlier benchmarks. I wonder that too. Maybe my laptop started throttling or something. :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy! Could you do the usual version stuff? Feel free to merge after that.
|
(Feel free to claim v0.38.3, I'll unrevert my other PR later.) |
|
I'm working on more, very similar performance improvements to VNV. I would fold them into the same release. |
* Change VNV to use Dict rather than OrderedDict
* Change concretisation from map(identity, x) to a comprehension
* Improve tighten_types!! and loosen_types!!
* Fix use of set_transformed!!
* Fix push!! for VarInfos
* Change the default element types in VNV to be Union{}
* In untyped_vector_varinfo, don't rely on Metadata
* Code style
* Run formatter
* In VNV, use typejoin rather than promote_type
* Bump patch version to 0.38.4
The VarNamedVector inner constructor has some checks to make sure that the data being fed in is consistent. This seems like a sensible idea. However, especially one of the checks is a bit expensive, and they were being called all the time. This PR turns them off when they are unnecessary, most notably when calling
unflatten.