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

deprecate dropgrad and ignore #1245

Merged
merged 5 commits into from
Jun 20, 2022
Merged

deprecate dropgrad and ignore #1245

merged 5 commits into from
Jun 20, 2022

Conversation

mzgubic
Copy link
Collaborator

@mzgubic mzgubic commented Jun 17, 2022

Contributes to #1235

I'll do nograd separately since it is used for a few adjoints that may need to be ported first.

@ToucheSir
Copy link
Member

Perhaps we could add a note in the docs about these deprecations pointing to CRC, or keep the autodocs and add pointers in the docstrings?

@CarloLucibello
Copy link
Member

The integration test failures could be real

@ToucheSir ToucheSir closed this Jun 20, 2022
@ToucheSir ToucheSir reopened this Jun 20, 2022
@ToucheSir
Copy link
Member

NeuralPDE failure is strictly on downstream and not us, kicking the tires again to see if DynamicPPL fixes itself per #1114 (comment).

@mzgubic
Copy link
Collaborator Author

mzgubic commented Jun 20, 2022

Perhaps we could add a note in the docs about these deprecations pointing to CRC, or keep the autodocs and add pointers in the docstrings?

Good idea, I've added a note to the docs.

The integration test failures could be real

NeuralPDE.jl seems to be failing with the same error as master.

DynamicPPL fails locally for me (but CI runs). Maybe @devmotion could advise whether there are any special requirements on running tests locally?

@devmotion
Copy link
Collaborator

Maybe @devmotion could advise whether there are any special requirements on running tests locally?

No, there shouldn't. I assume locally you don't run them with the latest release of DistributionsAD, most likely because you pinned or checked out some packages or (possibly unintentially) created a Manifest.toml file.

@mzgubic
Copy link
Collaborator Author

mzgubic commented Jun 20, 2022

DynamicPPL seems to have been fixed. Good to go? I don't have merge rights so please just merge when ready.

@ToucheSir
Copy link
Member

Done, thanks!

@ToucheSir ToucheSir merged commit a4d0ad4 into FluxML:master Jun 20, 2022
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.

4 participants