The Geometry class has a deleted copy-assignment operator. Why is this operator deleted? I tried to follow the git history but it end in a commit from 9y ago: 432cba1b that was imported from SVN. As far as I can tell, all real implementations from dune-geometry are copy-assignable.
Making a class not copy-assignable results in complicated code when we want to store a geometry in a class, e.g. in the implementation of grid-functions in dune-functions. There, we need to use a std::optional or pointer types to construct a new copy (copy construction seems to be still allowed). This has further consequences: The std::optional<Geometry> then is also not copyable.
I just want to understand the reasoning behind the deleted copy operator. Maybe it cannot be implemented efficiently (for some reason) in a specific grid manager?
Designs
Child items
...
Show closed items
Linked items
0
Link issues together to show that they're related.
Learn more.
I don't recall the original reasoning. One reason may be that a priori it is not clear whether such copying should be deep or shallow. Currently, copying a UGGridGeometry would result in a shallow copy only (this could be changed, however).
Note, copy-construction should still be possible. Just the copy-assignment is deleted. This means I can still construct std::optional<Geometry> but cannot copy this optional anymore. The user-declaration of operator=(Geometry const&) also prevents from any move operation to be implicitly declared.
Maybe your point that it is not ad-hoc clear what are the semantics of the copy-assignment makes it more safe to just delete it instead. But, maybe we could think about the correct semantics.
@oliver.sander I don't understand what you mean by 'shallow copy' here. As far as I can see, UGGridGeometry either contains a pointer to the persistent UG entity or derives from MultiLinearGeometry which stores a std::vector of coordinates. In both cases I would call this a deep copy.
BTW: In the latter case there's many reallocations of a std::vector: Two when updating the entity and one on each call of geometry(). Maybe this could be improved using ReservedVector.
I mean the case where UGGridGeometry contains only a pointer to the persistent UG entity. I would call copying such an object "shallow" because the "persistent UG entity" is not really persistent: It can change.
Maybe I'm wrong, but as far as I understood this points to a raw UG object that is not allocated or updated during iteration. This is what I meant by persistent. If this is the case, then that's a deep copy. If this is not the case, then UGGridGeometry would violate the general lifetime guarantee for Geometry objects (as long as the grid is not modified).
I have to ask again since this questions really bothers me.
A geometry is actually just an interface for a differentiable function. It might need some coordinate vectors for the parametrization, but thats it. A functions should be copyable (and moveable). This should also not be too expensive. A set of coordinate vectors should be fine to copy and it should never be a shallow copy (in my opinion).
What I also do not understand is: why is a geometry class copy-constructible but not copy-assigneable. Typically, these two operations should behave the same. If not there must be a strong reason (and an explanation) why.
Since a geometry is like a function, I want to store multiple geometries and maybe compose geometries. If the geometry is not copyable, I'm not sure whether this is allowed and safe and this would make some implementation much more complicated.
This must be for historical reasons. Geometries can exist longer than an entity that the geometry was obtained from but should not be stored in std::vector etc. I guess the original idea was to prevent writing of slow code. Not sure if this really is a good argument anymore.
@simon.praetorius: I think the main problem was that a geometry (as far as I remember) is not default constructible. Therefore the std::optional needs to be used.
Maybe we can bring up default construction next. This would make geometries semi-regular and solve the mentioned issue. Using any methods (except copy- or move-assignment) of a default-constructed geometry would then be UB until it's assigned with a non-default-constructed state.
it's the same as already specified for Entity (and probably other interface wrappers), no? There, the other member functions are also UB until a non-default-constructed entity is assigned.
The default geometry implementations could easily be made default constructible. It might be that it is a bit more difficult for the mapped geometry types. There we might need to require that e.g. lambda functions are default constructible (this requires c++20 (?)), or we use wrapper classes like std::optional for these.
You are right. There are workarounds for this that could be considered in the implementation, e.g. std::optional or std::function or something similar. If the Geometry is required to be default-constructible (I would like this) then the corresponding implementation could handle this.