Implement Lees-Edwards boundary conditions #4457
Conversation
Co-authored-by: Sebastian Bindgen <info@bindgen.net> Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
|
Note: this PR blocks the |
| public: | ||
| LinearShear() | ||
| : m_protocol{ | ||
| new ::LeesEdwards::ActiveProtocol{::LeesEdwards::LinearShear()}} { |
There was a problem hiding this comment.
For all the shear-cases, is there a reason to use new instead of std::make_shared for the construction of the core object? Intuitively i would expect it to work out of the box and to be more explicit than this solution.
There was a problem hiding this comment.
This new construction is actually quite common in the script interface. Unfortunately, the reasons for this design decision were not documented, as far as I know. The resulting shared_ptr is much smaller since it only contains a pointer instead of the pointee, and the rules for deallocation are slightly different (I think the pointee is deallocated immediately when the reference counter is zero, regardless of the value of the weak pointer counter).
There was a problem hiding this comment.
To me it is not clear where this difference comes from, especially because we just construct a shared-pointer from it. I would have expected that std::make_shared does exactly that, but I never bothered to check what std::make_shared actually does internally. I always had the impression that it is designed to replace new with a shared-ownership, which does not seem to be the whole story.
There was a problem hiding this comment.
The std::shared_ptr constructor takes a raw pointer as argument, and takes ownership of it, i.e. we are no longer allowed to call free() or delete on the original raw pointer. The resulting shared pointer is lightweight, since it only wraps a raw pointer. Whether the pointee is allocated with new or malloc is not really relevant, but the new syntax is a bit more readable. This strategy has some overhead due to the double dereference, but then we can have a lightweight std::vector<std::shared_ptr<T>> that is easy to resize and resort.
The std::shared_ptr copy constructor is designed to work with std::make_shared, which embeds the pointee. For our use case in the script interface, I think it's safe to replace the new syntax with the std::make_shared everywhere. It also bugged me a few weeks ago when I realized we were using the new syntax in the script interface.
Fixes #2051
Fixes #2083