JuliaReach / MathematicalSets.jl

Set definitions in Julia
https://juliareach.github.io/MathematicalSets.jl/
MIT License
5 stars 0 forks source link

Release #23

Open blegat opened 1 year ago

blegat commented 1 year ago

Hi @mforets and @schillic. Sorry for being so slow at this but I would like to finally start integrating this package. I think what's hard for this package is to decide what could be on it. The obvious candidates are space_dimension and dimension since that is what is needed by MathematicalSets to check that he sets he receives have the right dimension. For volume and surface_area, there might be bikeshedding on what it would mean for lower-dimensional objectsbut we can keep it as zero for these functions and not define any other function for these lower-dimensional objects for now. Maybe we can keep it simple at first, keep these 4 functions and add MathematicalSets as a dependency for LazySets, SemialgebraicSets, Polyhedra, SetProg, ... Once this is done, we can add it to MathematicalSystems and use it to check input. Then we'll see later one if we can integrate more things one by one but a small packages is also fine.

About union, intersections, etc..., if LazySets plans to support nonconvex sets at some point then it's best to leave these lazy operations in LazySets and we don't have to define them here and Dionysos/SetProg will use LazySets for that.

One other set that could be useful is SemialgebraicSets.FullSpace because I often need it and having SemialgebraicSets as dependency for that is a bit silly. In SemialgebraicSets, there is no dimension but we could give one so that we can implement dimension. Of course, we could decide whether it is R^n or C^n but we may just leave if ambiguous for now, it's just a set such that Base.in always returns true, Base.intersect is a no-op and Base.union absorbs. This could be use as returned value for state_set of unconstrained systems for instance.

What do you think ? If you are fine with it, I can make a PR.

schillic commented 1 year ago

Sounds good!

space_dimension and dimension

In LazySets we use dim and do not distinguish the two dimension concepts. But that is okay.

volume and surface_area

In LazySets we have volume, which generalizes to n dimensions (not sure if this is the intended semantics here). We also have area, but this is just volume for 2D sets (frankly, I think we should redefine that in LazySets as just a convenience alias). So LazySets does not have the functionality for surface_area at the moment.

I saw that MathematicalSets has an abstract type AbstractSet. Should we use a trait instead? I think they are more flexible and now is a good time to rethink the package.

SemialgebraicSets.FullSpace is probably the same as LazySets.Universe, except that the Universe stores its dimension. But since LazySets uses a type hierarchy instead of traits, I would prefer to keep it in LazySets. But then we could also have an EmptySpace type, and where do we stop? I think it is nice to keep packages as small as possible and not let the interface package provide an implementation. Maybe we can instead have a small standalone package for FullSpace and then optionally have another convenience package that just reexports both packages. The only problem with this approach is to find names for these packages. FullSpaceSet sounds a bit clumsy, but it would probably have to have a prefix or suffix to make it identifiable in the zoo of Julia packages. Does it make more sense to have unexported submodules for implementations as a compromise? Then there could then be a submodule MathematicalSets.Implementations that loads all implementations... (Related to that, I also had the idea to break up LazySets into smaller sub-packages, one for each set type, and let LazySets just be the cluster package. But that has the same problem.)

schillic commented 1 year ago

Let me add that I dislike that the dimension interface function can mean different things in different packages (see the quote below). That is exactly not what you want from an interface function. I suggest we keep having two different functions (space_dimension and dimension) and each of them have a fixed meaning. To make the meaning extra clear, it might be good to rename dimension. (Maybe we had a discussion leading to the current state in the past, but I forgot it.)

https://github.com/JuliaReach/MathematicalSets.jl/blob/7a02c7fb74813fb510d993c8807f8c4657311dda/src/topological.jl#L39-L46

blegat commented 1 year ago

We could drop dimension for the first release, it's not needed by MathematicalSystems and we can add it when we are done bikeshedding because it's a tricky one.

schillic commented 1 year ago

I saw that MathematicalSets has an abstract type AbstractSet. Should we use a trait instead? I think they are more flexible and now is a good time to rethink the package.

Here is an example: The IntervalArithmetic types Interval and IntervalBox describe sets. But they live in other packages, so we cannot make them <: AbstractSet. If instead we use a trait, we can. We have precisely that problem in LazySets (the LazySets.Interval is just a wrapper for IntervalArithmetic.Interval that is <: LazySet).

mforets commented 1 year ago

Since neither dimension nor space_dimension are used in LazySets, can't we keep the status quo by defining

# LazySets.jl
MathematicalSets.space_dimension(X::LazySet) = dim(X)
mforets commented 1 year ago

What do you think ? If you are fine with it, I can make a PR.

I like what you propose. Making this library a lightweight interface for set computations is still relevant IMO.

blegat commented 1 year ago

What would the trait accomplish ? Just check whether something is a set ? If this package is lightweight enough, it could also be considered as an dependency to IntervalArithmetics (@dpsanders what do you think ?) I guess the main issue with space_dimension is what we do for scalar sets, matrix sets and so on. We might need to do the same distinctions than size vs length for AbstractArray.

About having many small packages, it might be a bit cumbersome indeed. Would it be possible to have LazySets.Universe moved here as well as some part of the type hierarchy. If we could have one or two abstract types, some concept of space dimension, the universe and maybe an empty set, it could be good enough for this small package.