Assigning UUID per object
a UUID per object makes it possible to keep track of elements in complex optical systems and to add properties that should not be stored in the object struct, such as the color used to display the object.
@alfredclwong we probably want a map from symbols to UUID's and then another map from UUID's to objects. Do we want to detect duplicate and print a warning or throw an exception?
@BrianGun the more I think about it, the more I think we'll have to bite the bullet and add an ID field to every relevant type.
For immutable types, there is no way to distinguish programmatically between two objects if they have the same values. This will happen a lot since we are working with MLAs. Because of this, it'll be difficult to create a well-defined mapping from objects to IDs/properties if the ID isn't an explicit field. And because of immutability and how Julia handles inheritance, this unfortunately means we'll have to rewrite every constructor to create an ID at instantiation. This is tedious but not terrible, in my opinion.
Once we have this, it'll then be easy to define a Session type which holds properties::Dict{ID, Properties} to allow us to append draw properties etc. to each object. We don't even need to store the same IDs as provided by the constructors as Julia has a hash function which will create a unique hash for unique instances.
I just had a look at @cdhf's code and comments. The approach is basically what I've outlined above, although there are a few implementation details that I would change:
- Define our own
uuidfunction which points to an existing stdlib uuid call, e.g.uuid = uuid4(Random.GLOBAL_RNG). This makes it easier to maintain in the future if we find that one of them is not thread-safe etc. - Remove the requirement for detectors to have names. I would consider this an "aesthetic" feature, similar to setting the color of each surface in the visualization; as far as the core logic is concerned, it's unnecessary bulk. Instead, we can include this in the properties dictionary that we are constructing using the ID fields.
- Discuss what really makes a detector. As far as I can see, the
HierarchicalDetectormight as well just be the one and onlyDetectortype. Are there any other use cases that we can think of? - Remove the
detectorfield fromCSGOpticalSystem(definitely) and also removedetector_ids::AbstractVector{<:UUID}(maybe). Again, I think we can leverage the properties dictionary to define if a surface is a detector, as well as the actual detector properties.
@alfredclwong HierarchicalDetector{T<: Number} can do Real, and Complex which is good enough for a lot. But I could imagine wanting a detector that would store all the rays that intersect a particular pixel. Or the ids of the emitters that were the source for all the rays that intersected a pixel. Our system doesn't currently store enough information to do the latter but it would be possible to do the former. @cdhf and I discussed this problem of what a detector should be. For now we should make it as flexible as possible without requiring extensive rewrites. Err on the side of simplicity and minimizing programming effort.
@alfredclwong if you remove detector from csgoptical system then there isn't much reason for CSGOpticalSystem to exist. It's a container to group detectors and lens assemblies which we should replace with something else more general that lets you group multiple emitters and detectors and even lens assemblies. Although the last point we could probably punt on for now and just go with one lens assembly.
@BrianGun I'll leave the detector stuff for now then, it can wait.
The only "clever" way I can think of to augment types with an ID field is to define an Object wrapper with an ID field (see https://github.com/microsoft/OpticSim.jl/blob/alfred/id/src/ID.jl). This allows us to preserve existing functionality whilst writing new methods for Object{T} usages. However, this kind of mixin behaviour is very difficult to implement in Julia. As far as I know, we'll have to forward every method for every type, e.g.
surfaceintersection(surfaceobject::Object{Surface{T}}, ray::AbstractRay{T}) where T = surfaceintersection(surfaceobject.object, ray)
We could use this library if we only had to wrap one type, but since we want to at least cover all Primitives, it gets a lot more difficult. A good example of what I mean is shown here, where a metadata Dict is added to DataFrame. This is basically what we want to do, on a smaller scale.
Have you had any other thoughts on this?