Correct domain calculation for compose_trans #408
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR corrects the domain calculations for
compose_trans()
, which appear to be incorrect.The problem
Currently,
compose_trans()
pushes the domain of the first function through all transformations to calculate the domain. This procedure, when it works (i.e. if the range of each transformation in the chain is contained within the domain of the next one), yields the range of the composed set of transformations, not the domain.This leads to incorrect domains; e.g.:
The correct domain here is just the domain of
sqrt()
, which isc(0, Inf)
. We can see this by plotting it:This also leads to
compose_trans()
incorrectly erroring on valid compositions; e.g.:Yet, this should be a valid transformation, just with the domain
c(-Inf, 0)
instead of the original domain ofc(0, Inf)
(this also offers a solution to the question about reverse log in #393: the transformation is valid so long as the domain is correctly calculated).The solution
This PR adjusts the domain calculation to push the first domain forward through the transformations, intersecting it with the domains of each transformation along the way, then pushing the resulting range back through the inverses to get the domain.
On the above two examples, with this PR, we have:
and