JuliaApproximation / DomainSets.jl

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

Incorrect statement in README? #106

Closed bgroenks96 closed 10 months ago

bgroenks96 commented 2 years ago

The README states:

For example, the domain of the function log(x::Float64) is the infinite open interval, which is represented by the type HalfLine{Float64}().

But HalfLine is closed from the left, while the domain of log is open (0,Inf). It would be nice to have provided types for the strictly positive and strictly negative domains.

daanhb commented 2 years ago

I guess we can either change the README or the implementation. Is it a purity question or would it be helpful to have an open half line type?

dlfivefifty commented 2 years ago

Can't we just use

julia> Interval{:open, :closed}(0, Inf)
0.0..Inf (open–closed)
daanhb commented 2 years ago

That's appealing, but in that case it would be good to check that the interval code can deal with infinities

dlfivefifty commented 2 years ago

It does deal with infinities....

bgroenks96 commented 2 years ago

Yes you can specify it manually with IntervalSets, but it's such a common use case that I think it would be nice to have it. Something like Nonnegative StrictlyPositive (not HalfLine since I think this technically is closed on the left) would be a bit nicer than OpenInterval(0,Inf).

And I don't think Interval{:open, :closed}(0, Inf) makes sense because infinity should always be open, right?

Edit: Oops, Nonnegative would include zero, scratch that.

daanhb commented 2 years ago

It does deal with infinities....

Well you're right, it does. So we could do away with the special types altogether, rather than introducing more?

bgroenks96 commented 2 years ago

I don't know, I like the idea of having special case types. It can make the meaning of the interval more immediately clear.

daanhb commented 2 years ago

I was worried about the implementation effort, but I see that HalfLine already is implemented as an interval. That makes it "just work" and probably not hard to generalize. The completeness exercise consists in making sure that the special type itself is returned in as many cases as possible, rather than the corresponding generic interval. Apart from clarity, a tangible difference between having a special type and not having it is that one can dispatch on a domain being the half line. I don't know if anybody would do that for the open half line... For the time being, I'll change the readme to return the correct interval.

daanhb commented 2 years ago

For completeness, if you use a domain a lot in a certain application and want to make it its own type for better clarity, it's easy to do by wrapping it. See the "ExampleNamedDomain" here for an example. The idea is that the wrapped domain behaves just like the domain it is wrapping, but it has its own type. (I haven't used this much so not sure if it is working as intended - I'd be happy to fix issues.)

daanhb commented 2 years ago

I did away with the example altogether. It was the very first paragraph of the README and looked out of place there anyway. I think that closes the original issue, but leaves the decision to implement the open half line.

daanhb commented 2 years ago

With #107, just like balls and simplices, the positive and negative half lines can now be open or closed at the point zero. It is just HalfLine{T,:open} and HalfLine{T,:closed}, with closed the default for the positive halfline and open the default for the negative one. That makes everything syntactically compatible with what we had.

I did not give them names. Perhaps the names could be: PositiveRealLine, StrictlyPositiveRealLine, NegativeRealLine and StrictlyNegativeRealLine?

bgroenks96 commented 2 years ago

I like that solution!

I am kind of 50/50 now on whether or not we need the aliases... but if you add them, I would suggest PositiveRealLine for HalfLine{T,:open}, NonnegativeRealLine for HalfLine{T,:closed}, NegativeRealLine for NegativeHalfLine{T,:open}, and NonpositiveRealLine for NegativeHalfLine{T,:closed}; this would be most consistent with the standard mathematical terminology, I think.

We could also consider leaving these aliases un-exported. This way they are available for those who would like to use them but do not pollute the exported namespace.

daanhb commented 2 years ago

Question for @dlfivefifty: does turning HalfLine{T} into HalfLine{T,C} constitute a breaking change you think? Creation still works as before: HalfLine() == HalfLine{Float64}() == HalfLine{Float64,:closed}() That is why the open/closed parameter C is second.

The only problem is if someone assumes that HalfLine{T} is a concrete type, because it is now abstract and HalfLine{T,C} is concrete. I've checked all of github :-) and nobody makes that assumption.

Otherwise we can make v0.6. I don't have the time to make the other changes I had in mind for 0.6 (taking out everything with maps), so that would become a v0.7 goal.

daanhb commented 2 years ago

Let's make it v0.6. Fixing the set difference of open and closed intervals (#49) also leads to some behavioural changes that may break code.