search

TuringLang / docs

Documentation and tutorials for the Turing language
https://turinglang.org/docs/
MIT License
229 stars 99 forks source link

Add assertion to tutorial 0 and workaround for thread-unsafe Pkg.jl #278

Closed rikhuijzer closed 3 years ago

rikhuijzer commented 3 years ago

This PR adds an assertion (as part of #218).

Also:

review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

rikhuijzer commented 3 years ago

The safe instantiate now uses multiple processes.

I've also deleted some old files. I was planning to do that later in a separate PR, but I accidentally committed some of these files. Let me know if I should revert that.

rikhuijzer commented 3 years ago

@devmotion, I've enabled --depwarn=yes now in the CI which might avoid committing deprecated tutorials in the future. I've also enabled --depwarn=yes now on my pc by default.

Can we merge this PR? I've opened an issue for the deprecation and plan to pick that one up soon in anther PR.

devmotion commented 3 years ago

The changes to the Gaussian mixture model files are not mentioned above and seem unrelated?

devmotion commented 3 years ago

@devmotion, I've enabled --depwarn=yes now in the CI which might avoid committing deprecated tutorials in the future. I've also enabled --depwarn=yes now on my pc by default.

Can we merge this PR? I've opened an issue for the deprecation and plan to pick that one up soon in anther PR.

I don't think we should do it, I would suggest reverting these changes. It is completely fine to use and run the deprecated code for users (and us). The main point of these deprecations is that the code still works, it's just not future proof and we might have to update it when updating to an upcoming breaking release of Distributions. But then we'll notice that it has to be updated since it will be broken. Enabling deprecation warnings just increases build times (potentially even causes timeouts, we've observed this in Turing) and creates noisy outputs - and we still won't notice them if we do not carefully inspect the logs or outputs.

That being said, I think if we update or add tutorials it would be strange to use the deprecated version if we are aware of the deprecations since we have to update them at some point eventually (since I introduced these deprecations I noticed the deprecated implementation 😂).

rikhuijzer commented 3 years ago

The changes to the Gaussian mixture model files are not mentioned above and seem unrelated?

See above, where I said: "I've also deleted some old files. I was planning to do that later in a separate PR, but I accidentally committed some of these files. Let me know if I should revert that."

Enabling deprecation warnings just increases build times (potentially even causes timeouts, we've observed this in Turing) and creates noisy outputs - and we still won't notice them if we do not carefully inspect the logs or outputs.

Good to know. It seems to have already happened at 4263aab. I canceled that run.

That being said, I think if we update or add tutorials it would be strange to use the deprecated version if we are aware of the deprecations since we have to update them at some point eventually

Agreed. I'll keep https://github.com/TuringLang/TuringTutorials/issues/279 open then.

rikhuijzer commented 3 years ago

Ah I missed this completely :grimacing:

No problem. Thanks for the review :+1: