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

Drop unnecessary depwarn + fix Turing tests #433

Merged
merged 2 commits into from
Nov 11, 2022

Conversation

torfjelde
Copy link
Member

This PR fixes the performance regressions seen for Emcee in TuringLang/Turing.jl#1900.

@yebai @devmotion This should be an easy merge.

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

I'm curious, where and how do we hit this function? The deprecation warning seems to indicate we shouldn't call it and (maybe) we should rather fix wherever we call it?

@torfjelde
Copy link
Member Author

Now link! and invlink! are just depwarn followed by a call to _link! and _invlink!, respectively. When implementing this, I mistakenly also included the depwarn from the link! in one of the _link! implementations. So we're hitting it because I made a typo.

@devmotion
Copy link
Member

Ah OK, now I understand 🙂

@torfjelde
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Nov 11, 2022
This PR fixes the performance regressions seen for `Emcee` in TuringLang/Turing.jl#1900.

@yebai @devmotion This should be an easy merge.
@bors
Copy link
Contributor

bors bot commented Nov 11, 2022

Build failed:

@torfjelde
Copy link
Member Author

Cause of tests breaking: JuliaDiff/ForwardDiff.jl#606

@yebai
Copy link
Member

yebai commented Nov 11, 2022

Please feel free to merge it manually - you need to temporarily change GitHub setting to do that.

@torfjelde
Copy link
Member Author

I don't think I have the rights to do that @yebai ? At least I can't find it here in the project.

@yebai yebai merged commit 217b8cc into master Nov 11, 2022
@yebai yebai deleted the tor/remove-unnecesary-depwarn branch November 11, 2022 17:19
@yebai
Copy link
Member

yebai commented Nov 11, 2022

I merged it manually - there is an option "Do not allow bypassing the above settings" in branch protection. If you disable that, it allows you to merge PRs by hand.

alyst pushed a commit to alyst/DynamicPPL.jl that referenced this pull request Mar 21, 2023
* drop unnecessary depwarn from _link!

* bump patch version
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.

3 participants