search

JuliaApproximation / DomainSets.jl

A Julia package for describing domains as continuous sets of elements
MIT License
72 stars 12 forks source link

Use `Set` in `UnionDomain` to avoid redundancy and ordering #9

Closed dlfivefifty closed 5 years ago

dlfivefifty commented 7 years ago

It's better to use Set since this avoids redundant domains, and also makes == easy to implement.

dlfivefifty commented 7 years ago

This has been implemented in approxfun-support, foolishly, since it's just going to make things harder... but it's definitely the right approach I think: I can't see any reason for UnionDomain to benefit from having a Tuple or Vector of domains over a Set.

dlfivefifty commented 7 years ago

Indeed, foolish: I reverted iit in approxfun-support, since ApproxFun's PiecewiseSpace needs to have an ordering in order to interpret the coefficients, and in places in the code it assumes this ordering is consistent with the ordering of UnionDomain.

I might try making this change again as it still feels "right" for it to be a Set. ApproxFun can work around this by calling domain.(s.spaces) instead of domain(s).domains.

dlfivefifty commented 7 years ago

https://github.com/JuliaLang/julia/issues/23746 also makes working with Sets annoying, however

daanhb commented 7 years ago

Currently, the type of the list of domains is a parameter (DD), so it can be chosen freely. The idea was to allow both tuples and vectors: a tuple would be fully typed, making the union domain fast. The vector could allow for a long list of subdomains, but it would be fast only if they all had the same type. Yet, I don't think we ever relied on a UnionDomain being fast.

Set sounds like it would make the code shorter and clearer, which is good. I think we currently also have some code that enumerates over the subdomains, but I'd be surprised if the ordering matters anywhere - it shouldn't and if it does we can fix it.