Add inverse and simplify type structure

[devmotion]

This PR implements the suggestion in #43 (comment).

More concretely, if one defines GPLikelihoods.inverse(::typeof(g)) = f (probably one wants to define the reverse definition as well 😉) then one gets inv(::Link{typeof(g)}) isa Link{typeof(f)} for free. This allows to define LogLink, ExpLink just as aliases of Link.

(As a hidden feature, in addition to existing links the PR also adds the suggestion in the linked comment for log1pexp and logexpm1, i.e., Link(log1pexp) and Link(logexpm1) are invertible now as well.)

In the future, maybe definitions for inv(exp) etc. might be available in base (JuliaLang/julia#42421) or as type piracy in some package which would make this approach even more composable and put less maintenance burden on us.

[codecov]

Codecov Report

Merging #48 (d221863) into master (b39b3d8) will increase coverage by 0.27%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   83.33%   83.60%   +0.27%     
==========================================
  Files           7        8       +1     
  Lines          66       61       -5     
==========================================
- Hits           55       51       -4     
+ Misses         11       10       -1     

Impacted Files	Coverage Δ
src/links.jl	80.00% <90.90%> (-7.50%)	⬇️
src/inverse.jl	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b39b3d8...d221863. Read the comment docs.

[theogf]

Looks good! Should be easier to create new links now!
Would that make sense to create a macro for creating named links? like

@link CosLink cos
# returns
# const CosLink = Link{typeof(cos)}
# CosLink() = Link(cos)

[devmotion]

Would that make sense to create a macro for creating named links?

I would rather want to deprecate the aliases to be honest. I think one should just use Link(cos) and there should not be a need for a specific type alias.

[theogf]

I have to say I find them quite needed.
I'd rather see Likelihood{<:LogisticLink} then Likelihood{<:Link{typeof(logistic)}} (which also implies one would need to have logistic in the workspace.

[devmotion]

I'd rather see Likelihood{<:LogisticLink} then Likelihood{<:Link{typeof(logistic)}}

You could save 2 characters and write Likelihood{Link{typeof(logistic)}} 😄

which also implies one would need to have logistic in the workspace

Or you just use Likelihood{Link{typeof(GPLikelihoods.logistic)}} 🤣

I still wonder if we should really define them in GPLikelihoods or if it would be sufficient if users or developers define their own aliases if they want to dispatch on it multiple times. At least I am not a big fan of the "constructors" which I kept to make the PR non-breaking. I see that it is appealing to dispatch on ::CosLink instead of Link{...} (but again I'm not sure if this should be our responsibility) but I don't see a clear advantage in writing CosLink() instead of Link(cos). But if we don't define the constructor I think the macro is not needed and one could just define the alias as const CosLink = Link{typeof(cos)} manually.

[theogf]

You could save 2 characters and write

For once it's not even about the number of characters but more about readability 😆

I can see your point with the constructor though, but also I don't really see the negative effect of it either... Except for code everytime we want to create a new link...

[devmotion]

but also I don't really see the negative effect of it either...

It's more definitions, more work for the compiler 🤷 But I guess mainly it's my desire to provide a cleaner API where links for functions should always be constructed with Link(f). Maybe I should open an issue so we can discuss the API design with e.g. @willtebbutt and @st-- separate from this PR (which does not affect the API)?

[oschulz]

ToDo: Switch to InverseFunctions.jl. ;-)

[devmotion]

There's already an issue 😉

We can't switch before LogExpFunctions and StatsFuns support it though (if we don't want to introduce type piracy).

[oschulz]

We can't switch before LogExpFunctions and StatsFuns support it though (if we don't want to introduce type piracy).

Ah, right. I can get on that as soon as we have a ChangesOfVariables v1.1 with integrated test function out.