Skip to content

Fix testing failure on julia 1.0#222

Closed
felixcremer wants to merge 3 commits intoJuliaMath:masterfrom
felixcremer:gradient_bug
Closed

Fix testing failure on julia 1.0#222
felixcremer wants to merge 3 commits intoJuliaMath:masterfrom
felixcremer:gradient_bug

Conversation

@felixcremer
Copy link
Copy Markdown

This removes the import of LinearAlgebra: gradient and exports the gradient function because it has been fully removed (see JuliaLang/julia#16113).
It also includes the changes of #221 to Random.srand.

Felix Cremer added 3 commits August 10, 2018 00:03
The gradient function was completely removed and not moved to LinearAlgebra as documented in the depwarn.
This import fails therefore.
This is needed, because we don't import the gradient function anymore.
@tomasaschan
Copy link
Copy Markdown
Contributor

Gradient seems to me like it's a too general concept to be "owned" by Interpolations.jl. Is there somewhere else we should import the name from to make sure we're extending the concept, rather than exporting a new one? (Or is import merging solved nowadays?)

@timholy
Copy link
Copy Markdown
Member

timholy commented Aug 11, 2018

No, import merging isn't solved. I don't think we should export it.

Copy link
Copy Markdown
Contributor

@tomasaschan tomasaschan left a comment

Choose a reason for hiding this comment

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

Since #221 is merged now, this needs rebasing anyway, and the changes to test/scaling/nointerp.jl in this PR can be discarded.

Where has LinearAlgebra.gradient moved? Could gradient be imported from the new location?

If not, I vote for completely removing the export of gradient (and, possibly, gradient!, gradient1 and the corresponding /hessian[!1]?/ functions, for consistency), and promoting qualified use of those methods instead. (This would have to go through a deprecation cycle, of course.)

@mkborregaard
Copy link
Copy Markdown

Is it correct that merge of this PR is what keeps this package back from 1.0 compatibility?

@tomasaschan
Copy link
Copy Markdown
Contributor

@mkborregaard Technically, no. But (one of) the problem(s) addressed by this PR is, namely that we need to decide how to handle that gradient is no longer exported from base.

@felixcremer
Copy link
Copy Markdown
Author

Everything this tries to solve is handled in #226

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