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

Approxfun support #11

Merged
merged 17 commits into from
Sep 20, 2017
Merged

Approxfun support #11

merged 17 commits into from
Sep 20, 2017

Conversation

dlfivefifty
Copy link
Member

@dlfivefifty dlfivefifty commented Sep 19, 2017

ApproxFun now passes all tests using Domains.jl, though there is still some work to be done.

This makes the following changes:

  1. Support convert(Domain{T}, d::Domain) for each of the current domains. We use this to support in with different types.
  2. Allow UnionDomain(::AbstractVector). This may be removed if Use Set in UnionDomain to avoid redundancy and ordering #9 goes ahead.
  3. Support == and hash for UnionDomain, using a Set to resolve the issue of ordering
  4. Remove + meaning union #7: Remove + and other synonyms for union
  5. Support arithmetic for intervals and UnionDomain
  6. Support, for example, convert(Domain, Float64), which now returns a FullSpace{Float64}()
  7. Implement setdiff for intervals and UnionDomain
  8. Reserve Point for a Domain? #6: Point is now a domain
  9. Properly support open and closed endpoints for intervals in setdiff and union (TODO: intersect)
  10. Adds minimum, maximum, supremum and infimum. (TODO: maximum(norm, ::Domain) , etc., for higher dimensions.) for UnionDomain and intervals.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.7%) to 94.915% when pulling 6c137cd on approxfun-support into c90debf on master.

@codecov-io
Copy link

codecov-io commented Sep 19, 2017

Codecov Report

Merging #11 into master will decrease coverage by 2.65%.
The diff coverage is 84.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
- Coverage   97.57%   94.91%   -2.66%     
==========================================
  Files          23       23              
  Lines         535      649     +114     
==========================================
+ Hits          522      616      +94     
- Misses         13       33      +20
Impacted Files Coverage Δ
src/domains/trivial.jl 92.59% <100%> (+1.28%) ⬆️
src/generic/setoperations.jl 79.68% <58.62%> (-17.99%) ⬇️
src/generic/domain.jl 75% <60%> (-10.72%) ⬇️
src/domains/simple.jl 90% <83.33%> (-10%) ⬇️
src/generic/arithmetics.jl 92.85% <90%> (+1.94%) ⬆️
src/domains/interval.jl 97.14% <96.72%> (-0.45%) ⬇️

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 c90debf...704723e. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.7%) to 94.915% when pulling 704723e on approxfun-support into c90debf on master.

Copy link
Member Author

@dlfivefifty dlfivefifty left a comment

Choose a reason for hiding this comment

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

Btw, setdiff is fixed in 0.7, but Set(::Set) is a no-op, so the fix is fine.

Copy link
Member

@daanhb daanhb left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Semantic changes to in are fine: it returns false now in some cases, rather than throwing an error, which is convenient. We may need some more checks for open and closed domains (in setdiff for intervals, for example), but for the time being we are not using that functionality.

@dlfivefifty dlfivefifty merged commit 9635457 into master Sep 20, 2017
@dlfivefifty
Copy link
Member Author

dlfivefifty commented Sep 20, 2017

I think in should never throw an error, a feature that this branch doesn't actually fix. For example,

1+im in interval(0,1)

throws an error as intervals can't be promoted to Complex128.

In the case of intervals, we would want something like an EmbeddedDomain:

# represent D embedded in `FullSpace{T}()`. 
struct EmbeddedDomain{D,T} <: Domain{T}
   domain::D
end

# default is to embed, probably need to check the types makes sense
convert(::Type{Domain{T}}, d::Domain) where T = EmbeddedDomain{typeof(d),T}(d)

# TODO: can we avoid a try block?
function indomain(x::T, d::EmbeddedDomain{D,T}) where {T,D}
try
   xT = convert(eltype(D), x)
catch InexactError
    return false
end

in(xT, d.domain)
end

@dlfivefifty
Copy link
Member Author

This is related to JuliaApproximation/ApproxFun.jl#267 which came up with integrating delta functions.

@daanhb
Copy link
Member

daanhb commented Sep 25, 2017

Apparently in never throws an error, so yes, I agree we shouldn't either. I was going to reply that the EmbeddedDomain was already possible since there are embedding maps and mapped domains. However, it wasn't working due to the way mapped domains were implemented. They stored the inverse of the map rather than the map itself. I've changed that in the better_maps branch. Domains can now be embedded in a higher-dimensional space, or in the space of a wider type.

That doesn't fix in yet, but it makes it possible.

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.

5 participants