Dear Boost community, The formal review of Emil Dotchevski's QVM library begins on 7th Dec and ends on 16th Dec. Boost QVM defines a set of generic functions and operator overloads for working with quaternions, vectors and matrices of static size with the emphasis on 2, 3 and 4-dimensional operations needed in graphics, video games and simulation applications. While it provides its own quaternion, matrix and vector types, it is designed to work primarily with user-defined types through user specializations of the q_traits, v_traits and m_traits templates. QVM's source code is available on Github: https://github.com/zajo/boost-qvm Full documentation is also viewable on Github: http://zajo.github.io/boost-qvm/ We encourage your participation in this review. At a minimum, kindly state: - Whether you believe the library should be accepted into Boost * Conditions for acceptance - Your name - Your knowledge of the problem domain. You are strongly encouraged to also provide additional information: - What is your evaluation of the library's: * Design * Implementation * Documentation * Tests * Usefulness - Did you attempt to use the library? If so: * Which compiler(s) * What was the experience? Any problems? - How much effort did you put into your evaluation of the review? More information about the review process can be found here: http://www.boost.org/community/reviews.html We await your feedback! Best regards, Adam Wulkiewicz
Hi, here my quick review based on the documentation. I will not partake in the current discussion of the swizzling syntax.
We encourage your participation in this review. At a minimum, kindly state: - Whether you believe the library should be accepted into Boost Not now, but at a later point surely.
* Conditions for acceptance This is not because the library is not relevant or useful, or because of bad design, but because it misses important functionality in the current state that would give it impact in the current ecosystem. "If I already have to use two competing point libraries, why should I additionally introduce qvm?" The scope must be broadened by including some advanced algorithms which make qvm useful in the ecosystem, also interoperability with already existing boost components must be established. Some of its functionality already exists in boost, which makes acceptance as a standalone library a bit odd. It could be worthwhile to merge qvm with another geometry related boost library to strengthen the links between the libraries.
- Your knowledge of the problem domain. I have got basic knowledge in robotics and graphic applications, some physical simulations
You are strongly encouraged to also provide additional information: - What is your evaluation of the library's: * Usefulness
* Design The design makes sense. However for many use cases the current bindings are too complicated and for the problem of interoperability of point types, the issue of order has to be addressed, e.g. if one library takes
One has to see this library in the context of current program design in robotics. Typically we have some robotic application, for example an arm that has some software to move its joints in space. On the other hand we have a physical simulation of the arm, the object it carries, forces that are applied to it, and often a graphical simulator showing the (simulated) position of the arm. These software pieces are developed by different groups which need a definition of a point or a matrix etc and somehow these parts have to be stitched together in an application so that they work. qvm has to be seen in this context as the 15th standard designed to unify the 14 other standards that are out there( https://xkcd.com/927/ ). I think it does a good job at it as it provides a reasonable abstraction layer and a set of operations which then can be used without taking the types of the arguments into account, so that points from the one library can be taken and easily used together with the points of another library. points as ARGB while the other provides them as RGBA, assigning points between the two libraries should automatically do the right thing and not assigning the R part of the one point to the A part of the other. Bindings should make this explicit as swizzling is always a source of confusion and the library should make it as simple as possible to make this automatic. In many cases, points are some POD type in one of the many conventions for which reinterpret_cast<T(&)[N]>(p) is a valid expression. The only thing that needs to be provided for such a type is the order of elements. Thus it would be really good if the library could provide, for these cases, a simple binding macro BOOST_QVM_VECTOR_3D(MyType,T,XYZ) //MyType is equivalent to a T[3] with convention X-Y-Z in memory My issue with the library is, that it provides the bindings for very basic operations, but no advanced functionality that makes it superior over the three other point definitions that some projects might already have. Most importantly, i miss functionality regarding sets of points which are stored continuously in memory e.g. a std::vector<My3DVector>. We can often gain a bit by using the fact that this is equivalent to some T[N*3] array and optimize operations for this case. Other operations that are used in these cases are also: transforming point sets between different formats, e.g. interleaving channels, removing channels(or creating a view that hides the channel)... further, i think it misses some more advanced math used in actual simulations, e.g. defining rotations with time steps based on angular velocities, for example as provided by screw theory. I am not sure of the following points regarding interoperability with existing boost parts: - boost::math has a quaternion class already. Could they be used as default? Do we need two competing concepts of quaternions? - what about user defined value types, e.g. high precision math? - interoperability with boost.units? can i have a vector of positions, a vector of velocities and a scalar timestep and have the type magic working properly? (Is this relevant?) - what about boost.geometry? Does qvm come with the necessary type traits? If a user has its own point type and wants to use it together with qvm and boost.geometry, does he have to provide type traits for both libraries? - would it work with odeint? any troubles on this side to expect? Here, a proper timestep mechanic could really help! Best, Oswin
Oswin, thank you for the thoughtful evaluation. I'll respond to a few of your points below. On Thu, Dec 10, 2015 at 9:52 AM, Oswin Krause < Oswin.Krause@ruhr-uni-bochum.de> wrote:
qvm has to be seen in this context as the 15th standard designed to unify the 14 other standards that are out there( https://xkcd.com/927/ ). I think it does a good job at it as it provides a reasonable abstraction layer and a set of operations which then can be used without taking the types of the arguments into account, so that points from the one library can be taken and easily used together with the points of another library.
I appreciate this comment, it's exactly what motivated the design of QVM. However, there is another side effect: because in QVM the operations are decoupled from the types, it becomes practical to emit many different types from various operations. This enables the use of what I call view proxies, e.g. rotx_m takes an angle as float const & and "reinterprets" it as a rotation matrix without any temps.
* Design
The design makes sense. However for many use cases the current bindings are too complicated and for the problem of interoperability of point types, the issue of order has to be addressed, e.g. if one library takes points as ARGB while the other provides them as RGBA, assigning points between the two libraries should automatically do the right thing and not assigning the R part of the one point to the A part of the other. Bindings should make this explicit as swizzling is always a source of confusion and the library should make it as simple as possible to make this automatic.
I'm confused, the order is explicit in the bindings, you could store vector elements as YXWZ if you want, or even not store all of them, e.g. you could store X, Y and Z and bind W as 1 or 0.
My issue with the library is, that it provides the bindings for very basic operations, but no advanced functionality that makes it superior over the three other point definitions that some projects might already have. Most importantly, i miss functionality regarding sets of points which are stored continuously in memory e.g. a std::vector<My3DVector>. We can often gain a bit by using the fact that this is equivalent to some T[N*3] array and optimize operations for this case.
This is a good point but I consider this outside of the scope of generic libraries like QVM because such gains are too tightly coupled with the physical layout of your data. An interface that lets you multiply 20000 matrices with maximum efficiency won't be nearly as friendly as QVM; its matrix and vector types will have all kinds of physical restrictions, alignment requirements, etc. which may not be reasonable in contexts that need to deal with only a few objects. Yet QVM lets you bind such uber-fast (and in, my experience, often platform-specific) types and use them together with more civilized, higher-level types.
further, i think it misses some more advanced math used in actual simulations, e.g. defining rotations with time steps based on angular velocities, for example as provided by screw theory.
This might make sense to add to QVM, I can't tell because that's outside of my expertise.
- boost::math has a quaternion class already. Could they be used as default? Do we need two competing concepts of quaternions?
The question do we want another quaternion, vector or matrix type is a good one, and the answer is that what what we want doesn't matter: as a matter of fact there are a ton of quaternion, vector and matrix types already. Actually QVM lets you define or emit 20 more tons of such types, I consider that a good thing. :)
- what about user defined value types, e.g. high precision math?
This is supported, see http://zajo.github.io/boost-qvm/scalar_requirements.html.
- what about boost.geometry? Does qvm come with the necessary type traits? If a user has its own point type and wants to use it together with qvm and boost.geometry, does he have to provide type traits for both libraries?
There is some overlap between Geometry and QVM but it is less clear if they can be unified or if that's a good idea. Thanks! Emil
*Please find attached my review of QVM library.* *Whether you believe the library should be accepted into Boost* *Conditionally Accepted into Boost* * * Conditions for acceptance* 1. The author should provide out of box versions of 2,3,4 dimensions vector matrix quaternion classes which can be readily imported by the user and used (without messing around with traits and such). 2. An effort should be made to make the APIs more compatible with current libraries such as Eigen and Armadillo. For instance take the norm operations : Both Eigen and Armadillo uses the norm()keyword for the norm and normSquared() (only in Eigen). However QVM uses mag and mag2 which is pretty non-intuitive. I would encourage Emil to explore this aspect so that the learning curve for the new library is less. 3. Compatibility with GLSL pipeline (like glm currently provides ) will be a big plus since it is the target demographic for this library. That said, this is a good library which emulates the functionality like *GLM* (http://glm.g-truc.net/0.9.1/api/index.html) with an array of features that both new users (not so much) and advanced users will find useful. However the target demographics for this library will be mostly graphics people. But I still believe this is a good thing for boost to have and it will nicely complement the existing linear algebra framework provided by uBLAS. *Your name* Rajaditya Mukherjee *Your knowledge of the problem domain.* I am a Phd student in Computer Graphics and I have been working with Matrices and Vectors for my research for the past 4 years. I am also aware of Eigen, uBLAS, armadillo, openCV and CGAL's matrix Vector implementations since I have used all of them in my research code. *You are strongly encouraged to also provide additional information:* - *What is your evaluation of the library's:* * * Design* I havent gone through the entire set of APIs but I have selectively ran the test files for the features of interest to me (such as Swizzling and Quaternions). However I have certain comments about the nomenclature of the principle APIs (which I have gone over in the Conditions for acceptance section). Another issue I have is with the matrix-matrix view operation. Currently it supports only single row and column deletion. I would like it if we can arbitrarily select a starting index and a certain number of rows and columns (imagine the view proxy operations in uBlas if you will) This limits the usefulness to a certain degree. * * Implementation* The implementation looks robust enough to support its claimed compatibility with generic types. * * Documentation* I have some issues with the nomenclature of APIs. I believe this is one of the conditions of my acceptance. ** Tests* The test battery is pretty comprehensive. However some of them require modifications to run in the new system because they use boost::test_tools::check_is_close which is depreciated in the current version as per this link ( http://lists.boost.org/boost-users/2015/09/85042.php). I would request Emil to update them as per the link so it runs without modification on newer boost systems. ** Usefulness* This will be useful for Graphics Applications to compute the various rendering matrices (MVP) that are needed in the graphics pipeline and other allied geometric transforms. *This is a very special purpose library. Users interested in deeper linear algebra applications will find uBLAS more to their liking. * *- Did you attempt to use the library? If so:* ** Which compiler(s)* MSVC 2013 ** What was the experience? Any problems?* There are certain trivial issues I encountered with MSVC. Emil resolved them pretty fast in the mailing list but right now, this library is not very MSVC friendly. Also, the test battery needs to be upgraded for newer boosts. (I had to manually change some header files and a couple of lines here and there). On a non-implementation note, there is a certain learning curve with this library. For instance the library absolutely needs the default constructor for certain operations (normalized in my case couldn't run without a default constructor, I will be happy to discuss the exact testing scenario that brought this comment as a followup). This should be mentioned clearly in the requirement for the generic types. (Maybe this is something Emil can clear up in the final version) *- How much effort did you put into your evaluation of the review?* 7 hours sporadically (not in a single sitting) *Rajaditya Mukherjee* *4th Year Graduate Student* Dept. of Computer Science and Engineering The Ohio State University Columbus, Ohio Tel :- +1-(614)-271-4439 email :- rajaditya.mukherjee@gmail.com <mukherjee.62@osu.edu>, raj@rajaditya.com <mukherjr@cse.ohio-state.edu> *Rajaditya Mukherjee * *3rd Year Graduate Student* Dept. of Computer Science and Engineering The Ohio State University Columbus, Ohio Tel :- +1-(614)-271-4439 email :- rajaditya.mukherjee@gmail.com <mukherjee.62@osu.edu>, raj@rajaditya.com <mukherjr@cse.ohio-state.edu> On Thu, Dec 10, 2015 at 4:06 PM, Emil Dotchevski <emildotchevski@gmail.com> wrote:
Oswin, thank you for the thoughtful evaluation. I'll respond to a few of your points below.
On Thu, Dec 10, 2015 at 9:52 AM, Oswin Krause < Oswin.Krause@ruhr-uni-bochum.de> wrote:
qvm has to be seen in this context as the 15th standard designed to unify the 14 other standards that are out there( https://xkcd.com/927/ ). I think it does a good job at it as it provides a reasonable abstraction layer and a set of operations which then can be used without taking the types of the arguments into account, so that points from the one library can be taken and easily used together with the points of another library.
I appreciate this comment, it's exactly what motivated the design of QVM. However, there is another side effect: because in QVM the operations are decoupled from the types, it becomes practical to emit many different types from various operations. This enables the use of what I call view proxies, e.g. rotx_m takes an angle as float const & and "reinterprets" it as a rotation matrix without any temps.
* Design
The design makes sense. However for many use cases the current bindings are too complicated and for the problem of interoperability of point types, the issue of order has to be addressed, e.g. if one library takes points as ARGB while the other provides them as RGBA, assigning points between the two libraries should automatically do the right thing and not assigning the R part of the one point to the A part of the other. Bindings should make this explicit as swizzling is always a source of confusion and the library should make it as simple as possible to make this automatic.
I'm confused, the order is explicit in the bindings, you could store vector elements as YXWZ if you want, or even not store all of them, e.g. you could store X, Y and Z and bind W as 1 or 0.
My issue with the library is, that it provides the bindings for very basic operations, but no advanced functionality that makes it superior over the three other point definitions that some projects might already have. Most importantly, i miss functionality regarding sets of points which are stored continuously in memory e.g. a std::vector<My3DVector>. We can often gain a bit by using the fact that this is equivalent to some T[N*3] array and optimize operations for this case.
This is a good point but I consider this outside of the scope of generic libraries like QVM because such gains are too tightly coupled with the physical layout of your data. An interface that lets you multiply 20000 matrices with maximum efficiency won't be nearly as friendly as QVM; its matrix and vector types will have all kinds of physical restrictions, alignment requirements, etc. which may not be reasonable in contexts that need to deal with only a few objects.
Yet QVM lets you bind such uber-fast (and in, my experience, often platform-specific) types and use them together with more civilized, higher-level types.
further, i think it misses some more advanced math used in actual simulations, e.g. defining rotations with time steps based on angular velocities, for example as provided by screw theory.
This might make sense to add to QVM, I can't tell because that's outside of my expertise.
- boost::math has a quaternion class already. Could they be used as default? Do we need two competing concepts of quaternions?
The question do we want another quaternion, vector or matrix type is a good one, and the answer is that what what we want doesn't matter: as a matter of fact there are a ton of quaternion, vector and matrix types already. Actually QVM lets you define or emit 20 more tons of such types, I consider that a good thing. :)
- what about user defined value types, e.g. high precision math?
This is supported, see http://zajo.github.io/boost-qvm/scalar_requirements.html.
- what about boost.geometry? Does qvm come with the necessary type traits? If a user has its own point type and wants to use it together with qvm and boost.geometry, does he have to provide type traits for both libraries?
There is some overlap between Geometry and QVM but it is less clear if they can be unified or if that's a good idea.
Thanks! Emil
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Rajaditya, thank you for the detailed review! Below I'll respond to a few points you made. On Sat, Dec 12, 2015 at 10:15 PM, Rajaditya Mukherjee < rajaditya.mukherjee@gmail.com> wrote:
*Conditionally Accepted into Boost* * * Conditions for acceptance* 1. The author should provide out of box versions of 2,3,4 dimensions vector matrix quaternion classes which can be readily imported by the user and used (without messing around with traits and such).
There are such class templates: quat<>, vec<> and mat<>. Or do you mean that you want to have specific typedefs like typedef vec<float,3> float3?
3. Compatibility with GLSL pipeline (like glm currently provides ) will be a big plus since it is the target demographic for this library.
I agree, there should be OpenGL and DirectX and other popular API bindings included in QVM.
The test battery is pretty comprehensive. However some of them require modifications to run in the new system because they use boost::test_tools::check_is_close which is depreciated in the current version as per this link ( http://lists.boost.org/boost-users/2015/09/85042.php). I would request Emil to update them as per the link so it runs without modification on newer boost systems.
Thank you for this note, will do.
*- Did you attempt to use the library? If so:* ** Which compiler(s)* MSVC 2013 ** What was the experience? Any problems?* There are certain trivial issues I encountered with MSVC. Emil resolved them pretty fast in the mailing list but right now, this library is not very MSVC friendly.
Could you clarify, what do you mean by it is not MSVC-friendly? I've been using QVM with MSVC for years without a problem except for the MSVC parsing bug you stumbled upon with (v,A<i>) (I've added that in the known issues page: http://zajo.github.io/boost-qvm/known_quirks_and_issues.html).
For instance the library absolutely needs the default constructor for certain operations (normalized in my case couldn't run without a default constructor, I will be happy to discuss the exact testing scenario that brought this comment as a followup).
Yes, I'm interested to understand what brought this comment, though I am not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value. It is possible to change normalized() to always return qvm::vec<>, which converts implicitly to any compatible vector type, but that would introduce a temp when assigning to a user-defined vector type. That seems like a bad idea. Thanks once again! Emil
On 12/13/2015 5:13 PM, Emil Dotchevski wrote:
Yes, I'm interested to understand what brought this comment, though I am not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value.
There's always the copy constructor, which I believe is already implicitly required by the library. Have you experimented with this before? Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
On Sun, Dec 13, 2015 at 1:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 5:13 PM, Emil Dotchevski wrote:
Yes, I'm interested to understand what brought this comment, though I am not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value.
There's always the copy constructor, which I believe is already implicitly required by the library. Have you experimented with this before?
Actually, there might not be a copy constructor. In QVM views are non-copyable. For example the return type of qvm::row and qvm::col, which present a row or a column of a matrix as a vector, return non-copyable types (which are still valid arguments for the normalized() function in question.) Regardless, the copy constructor can't be used to create a vector from 3 scalars. Perhaps I misunderstand you. Emil
On 12/13/2015 9:07 PM, Emil Dotchevski wrote:
On Sun, Dec 13, 2015 at 1:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 5:13 PM, Emil Dotchevski wrote:
Yes, I'm interested to understand what brought this comment, though I am not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value.
There's always the copy constructor, which I believe is already implicitly required by the library. Have you experimented with this before?
Actually, there might not be a copy constructor. In QVM views are non-copyable. For example the return type of qvm::row and qvm::col, which present a row or a column of a matrix as a vector, return non-copyable types (which are still valid arguments for the normalized() function in question.)
That's odd, views as I know them are copyable and cheaply so, since they are shallow. I would expect those views you mention not only to be copyable, but to be trivially-copyable. Could you expand on that? Anyways, I imagine calling `normalized` with one of those views either won't return something of that view's type, or it won't default-construct a view. Am I close here?
Regardless, the copy constructor can't be used to create a vector from 3 scalars. Perhaps I misunderstand you.
You say QVM can't invoke constructors other than the default constructor for user-defined types, but it already does call copy and move constructors already. Considering that, in the following call: myvec3 n = normalized(myvec3(1, 2, 3)); if the compiler says there is an error here because `normalized` needs a default constructor, I'm gonna start suspecting something wrong is going on with the library. This is what I would naively expect `normalized` to be doing: template <typename Vec> Vec normalized(Vec v) { return v /= length(v); } Now I vaguely recall something about being able to explicitly specify the return type, or specializing some kind of deduction facilities, or at least another place were one wouldn't have a source value to copy from, where a default construction would be unsurprising. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 9:07 PM, Emil Dotchevski wrote:
On Sun, Dec 13, 2015 at 1:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 5:13 PM, Emil Dotchevski wrote:
Yes, I'm interested to understand what brought this comment, though I am
not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value.
There's always the copy constructor, which I believe is already implicitly required by the library. Have you experimented with this before?
Actually, there might not be a copy constructor. In QVM views are non-copyable. For example the return type of qvm::row and qvm::col, which present a row or a column of a matrix as a vector, return non-copyable types (which are still valid arguments for the normalized() function in question.)
That's odd, views as I know them are copyable and cheaply so, since they are shallow. I would expect those views you mention not only to be copyable, but to be trivially-copyable. Could you expand on that?
In QVM views do not create temps, they are similar to static type casts and have zero abstraction penalty (assuming inlining works). For example, look at the "col" view, which accesses a matrix column as a vector: https://github.com/zajo/boost-qvm/blob/master/libs/qvm/include/boost/qvm/map.... Near the top of the selection you'll see a col_ template and its traits, then the col function itself looks like this: template <int Col,class A> typename boost::enable_if_c< is_m<A>::value, qvm_detail::col_<Col,A> &>::type BOOST_QVM_INLINE_TRIVIAL col( A & a ) { return reinterpret_cast<typename qvm_detail::col_<Col,A> &>(a); } The col_ class template itself isn't copyable because there is no object to copy -- the returned reference refers to the argument of the user-defined matrix type A used to instantiate the col function template.
Anyways, I imagine calling `normalized` with one of those views either won't return something of that view's type, or it won't default-construct a view. Am I close here?
Calling normalized() on a view returns a type deduced by deduce_v, the default for views being a compatible instantiation of the qvm::vec<> template. Such types are required to be copyable and to have accessible default constructor.
if the compiler says there is an error here because `normalized` needs a default constructor, I'm gonna start suspecting something wrong is going on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable. Emil
On 12/14/2015 5:53 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 9:07 PM, Emil Dotchevski wrote:
On Sun, Dec 13, 2015 at 1:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/13/2015 5:13 PM, Emil Dotchevski wrote:
Yes, I'm interested to understand what brought this comment, though I am
not sure that there is a solution. The problem is that QVM can't invoke constructors (other than the default constructor) for user-defined types, because different types could have different constructors (believe me, I've seen game programmers define some crazy matrix/vector constructors). Because functions like normalized() operate on user-defined types, they use the default constructor to make a local object before returning it by value.
There's always the copy constructor, which I believe is already implicitly required by the library. Have you experimented with this before?
Actually, there might not be a copy constructor. In QVM views are non-copyable. For example the return type of qvm::row and qvm::col, which present a row or a column of a matrix as a vector, return non-copyable types (which are still valid arguments for the normalized() function in question.)
Anyways, I imagine calling `normalized` with one of those views either won't return something of that view's type, or it won't default-construct a view. Am I close here?
Calling normalized() on a view returns a type deduced by deduce_v, the default for views being a compatible instantiation of the qvm::vec<> template. Such types are required to be copyable and to have accessible default constructor.
Nod, this is one of the unsurprising cases I mentioned. Reinstating some snipped context:
Now I vaguely recall something about being able to explicitly specify the return type, or specializing some kind of deduction facilities, or at least another place were one wouldn't have a source value to copy from, where a default construction would be unsurprising.
if the compiler says there is an error here because `normalized` needs a default constructor, I'm gonna start suspecting something wrong is going on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable.
Does this mean that the library explicitly supports movable-only types? That's somewhat unexpected, given that the library requires all type information necessary for embedded storage, but it's not a bad thing. It would be good to add this information to the reference documentation. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
On Mon, Dec 14, 2015 at 1:12 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 5:53 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé <
if the compiler says there is an error here because `normalized` needs a
default constructor, I'm gonna start suspecting something wrong is going on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable.
Does this mean that the library explicitly supports movable-only types? That's somewhat unexpected, given that the library requires all type information necessary for embedded storage, but it's not a bad thing. It would be good to add this information to the reference documentation.
QVM is neutral wrt move semantics but views are non-copyable and non-movable because copy or move semantics would require a temporary object, which QVM views are carefully designed to avoid. You can assign to a view and it will convert to any compatible type, but you can't copy or move it. That's why you can chain views up with no overhead: any operation on a view or multiple chained views works with the original object, no other objects are created. Emil
On 12/14/2015 7:57 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 1:12 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 5:53 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé <
if the compiler says there is an error here because `normalized` needs a
default constructor, I'm gonna start suspecting something wrong is going on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable.
Does this mean that the library explicitly supports movable-only types? That's somewhat unexpected, given that the library requires all type information necessary for embedded storage, but it's not a bad thing. It would be good to add this information to the reference documentation.
QVM is neutral wrt move semantics but views are non-copyable and non-movable because copy or move semantics would require a temporary object, which QVM views are carefully designed to avoid. You can assign to a view and it will convert to any compatible type, but you can't copy or move it. That's why you can chain views up with no overhead: any operation on a view or multiple chained views works with the original object, no other objects are created.
Views are special, that has already been established. My question was not in regard of views, whose case I said is unsurprising. My question was with respect to this simple guy requiring the presence of a default constructor: myvec3 n = normalized(myvec3(1, 2, 3)); Why does this guy require a default constructor? Now you said the library cannot call a constructor other than the default one, which is not the case. So I wonder if you had experimented with this particular case, which does not conceptually require a default constructor, and for which the library is already calling another constructor, a copy/move constructor given the simple fact that it is being returned from a function. Why does this guy require a default constructor? I understand now that it doesn't, you simply don't want to handle this case any different than you already do views. There's nothing wrong with that. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
On Mon, Dec 14, 2015 at 3:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 7:57 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 1:12 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 5:53 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé <
if the compiler says there is an error here because `normalized` needs a
default constructor, I'm gonna start suspecting something wrong is going
on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable.
Does this mean that the library explicitly supports movable-only types? That's somewhat unexpected, given that the library requires all type information necessary for embedded storage, but it's not a bad thing. It would be good to add this information to the reference documentation.
QVM is neutral wrt move semantics but views are non-copyable and non-movable because copy or move semantics would require a temporary object, which QVM views are carefully designed to avoid. You can assign to a view and it will convert to any compatible type, but you can't copy or move it. That's why you can chain views up with no overhead: any operation on a view or multiple chained views works with the original object, no other objects are created.
Views are special, that has already been established. My question was not in regard of views, whose case I said is unsurprising. My question was with respect to this simple guy requiring the presence of a default constructor:
myvec3 n = normalized(myvec3(1, 2, 3));
Why does this guy require a default constructor? Now you said the library cannot call a constructor other than the default one, which is not the case. So I wonder if you had experimented with this particular case, which does not conceptually require a default constructor, and for which the library is already calling another constructor, a copy/move constructor given the simple fact that it is being returned from a function.
Why does this guy require a default constructor? I understand now that it doesn't, you simply don't want to handle this case any different than you already do views. There's nothing wrong with that.
I'm not sure if we're on the same page. I'll try again. :) If you have struct myvec3 { .... }; then if you go: myvec3 v1 = ....; myvec3 v2=normalized(v1); It is indeed puzzling why would that need a default constructor. But consider this code: struct mymat3 { .... }; mymat3 m = ....; myvec3 v=normalized(col<1>(m)); //Take column 1 as vector In this case col<1> returns a reference to m, which is passed by reference to normalized. The use of col<1> created no temporary, but the flip side is that the reference it returns is of type that (by design) is neither copyable or movable. If instead you did: myvec3 v=normalized(col<1>(transp(m))); //Transpose, then take column 1 as vector transp returns a reference to m, which col<1> takes by reference and returns by reference, which is then passed by reference to normalized(), so again, no temporary is created in this sequence of calls. But in the end, normalized must return an object -- and it can't use a copy constructor (different types). And since I've never encountered a vector type without a default constructor, it seemed reasonable to not worry too much about requiring it. Emil
On 12/14/2015 9:07 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 3:16 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 7:57 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 1:12 PM, Agustín K-ballo Bergé < kaballo86@hotmail.com> wrote:
On 12/14/2015 5:53 PM, Emil Dotchevski wrote:
On Mon, Dec 14, 2015 at 4:46 AM, Agustín K-ballo Bergé <
if the compiler says there is an error here because `normalized` needs a
default constructor, I'm gonna start suspecting something wrong is going
on with the library. This is what I would naively expect `normalized` to be doing:
template <typename Vec> Vec normalized(Vec v) { return v /= length(v); }
It can't be implemented that way because the type of the argument might not be copyable.
[snip]
Views are special, that has already been established. My question was not in regard of views, whose case I said is unsurprising. My question was with respect to this simple guy requiring the presence of a default constructor:
myvec3 n = normalized(myvec3(1, 2, 3));
[snip]
Why does this guy require a default constructor? I understand now that it doesn't, you simply don't want to handle this case any different than you already do views. There's nothing wrong with that.
I'm not sure if we're on the same page. I'll try again. :)
[snip]
But in the end, normalized must return an object -- and it can't use a copy constructor (different types). And since I've never encountered a vector type without a default constructor, it seemed reasonable to not worry too much about requiring it.
Yep, that sums it up. There's cases were the need for a default constructor is artificial since a copy constructor could be used, but special-casing that is not worth the trouble. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
I will try to clarify the
*Conditionally Accepted into Boost* * * Conditions for acceptance* 1. The author should provide out of box versions of 2,3,4 dimensions vector matrix quaternion classes which can be readily imported by the user and used (without messing around with traits and such).
There are such class templates: quat<>, vec<> and mat<>. Or do you mean that you want to have specific typedefs like typedef vec<float,3> float3 ?
I will confess that I had missed that quat<> vec<> and mat<> were already present. I was hinting at that but the convinient typedefs would definitive be a plus. Libraries like Eigen and GLM also do it nad it would definitely enhance the readibility and "out-of-box" usability of the library.
*- Did you attempt to use the library? If so:* ** Which compiler(s)* MSVC 2013 ** What was the experience? Any problems?* There are certain trivial issues I encountered with MSVC. Emil resolved them pretty fast in the mailing list but right now, this library is not very MSVC friendly.
Could you clarify, what do you mean by it is not MSVC-friendly? I've been using QVM with MSVC for years without a problem except for the MSVC parsing bug you stumbled upon with (v,A<i>) (I've added that in the known issues page: http://zajo.github.io/boost-qvm/known_quirks_and_issues.html).
I thought that bug about the need for default consturctor was a MSVC thing. I have explained it below and if it is something which I am not understadning correctly, please point out to me. So essentially I wrote this small piece of code (same as one before) #include <boost/qvm/all.hpp> #include <iostream> class float4 { public: float4(float a, float b, float c, float d) { container[0] = a; container[1] = b; container[2] = c; container[3] = d; } float container[4]; }; namespace boost { namespace qvm { template<> struct v_traits<float4> { static int const dim = 3; typedef float scalar_type; template <int I> static inline scalar_type &w(float4 & v) { return v.container[I]; } template <int I> static inline scalar_type r(float4 const & v) { return v.container[I]; } static inline scalar_type &iw(int i, float4 &v) { return v.container[i]; } static inline scalar_type ir(int i, float4 const & v) { return v.container[i]; } }; } } using namespace boost::qvm; int main() { float4 vec(11.0, 0.0, 5.0, -6.0); 1----std::cout << mag(vec); 2----std::cout << mag(normalized(vec)); } So the issue is that line 1 works without the presence of default constructor but line 2 needs the same (more specifically the normalized in place keyword needs it). I weas just wondering if there is a generic way to enforce this behavior. In the usecases in your tests, most of them use the default constructor so it doesn't manifest itself too much. But I read your comment about returning a qcm::vec<> type and I do understand the problem you mentioned.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Sun, Dec 13, 2015 at 3:16 PM, Rajaditya Mukherjee < rajaditya.mukherjee@gmail.com> wrote:
I will try to clarify the
*Conditionally Accepted into Boost* * * Conditions for acceptance* 1. The author should provide out of box versions of 2,3,4 dimensions vector matrix quaternion classes which can be readily imported by the user and used (without messing around with traits and such).
There are such class templates: quat<>, vec<> and mat<>. Or do you mean that you want to have specific typedefs like typedef vec<float,3> float3 ?
I will confess that I had missed that quat<> vec<> and mat<> were already present. I was hinting at that but the convinient typedefs would definitive be a plus. Libraries like Eigen and GLM also do it nad it would definitely enhance the readibility and "out-of-box" usability of the library.
To clarify, these types ( http://zajo.github.io/boost-qvm/quaternion_vector_and_matrix_types_reference...) aren't provided just for convenience, they serve as the default return types for binary operations that are invoked with arguments of different static types. For example, if you multiply a user-defined type mat33 by a user-defined type matrix33, what type should the result be? In QVM, by default you're going to get a suitable instance of the qvm::mat<> template, which can be assigned to any compatible user-defined matrix type, including the types used as arguments. You can specialize the deduce_m2 template to specify a different type depending of the argument types.
** Which compiler(s)* MSVC 2013 ** What was the experience? Any problems?* There are certain trivial issues I encountered with MSVC. Emil resolved them pretty fast in the mailing list but right now, this library is not very MSVC friendly.
Could you clarify, what do you mean by it is not MSVC-friendly? I've been using QVM with MSVC for years without a problem except for the MSVC parsing bug you stumbled upon with (v,A<i>) (I've added that in the known issues page: http://zajo.github.io/boost-qvm/known_quirks_and_issues.html).
I thought that bug about the need for default consturctor was a MSVC thing.
No, it is not. I don't think that matrix or vector types without a default constructor exist in the wild, perhaps I'm wrong. So yes, at the very least I need to explicitly state the requirement for an accessible default constructor.
I have explained it below and if it is something which I am not understadning correctly, please point out to me. So essentially I wrote this small piece of code (same as one before) #include <boost/qvm/all.hpp> #include <iostream>
class float4 { public: float4(float a, float b, float c, float d) { container[0] = a; container[1] = b; container[2] = c; container[3] = d; } float container[4]; };
namespace boost { namespace qvm { template<> struct v_traits<float4> { static int const dim = 3; typedef float scalar_type;
template <int I> static inline scalar_type &w(float4 & v) { return v.container[I]; } template <int I> static inline scalar_type r(float4 const & v) { return v.container[I]; }
static inline scalar_type &iw(int i, float4 &v) { return v.container[i]; } static inline scalar_type ir(int i, float4 const & v) { return v.container[i]; } }; } }
using namespace boost::qvm;
int main() { float4 vec(11.0, 0.0, 5.0, -6.0); 1----std::cout << mag(vec); 2----std::cout << mag(normalized(vec)); }
So the issue is that line 1 works without the presence of default constructor but line 2 needs the same (more specifically the normalized in place keyword needs it). I weas just wondering if there is a generic way to enforce this behavior.
Yeah, I'd just add a default constructor to your float4 type. I believe you'll need it anyway outside of a toy example like this. Thanks! Emil
On Sat, Dec 12, 2015 at 10:15 PM, Rajaditya Mukherjee < rajaditya.mukherjee@gmail.com> wrote:
** Tests* The test battery is pretty comprehensive. However some of them require modifications to run in the new system because they use boost::test_tools::check_is_close which is depreciated in the current version as per this link ( http://lists.boost.org/boost-users/2015/09/85042.php). I would request Emil to update them as per the link so it runs without modification on newer boost systems.
Thanks for pointing out this issue, it's fixed now. Emil
Oswin Krause wrote:
We encourage your participation in this review. At a minimum, kindly
state: - Whether you believe the library should be accepted into Boost Not now, but at a later point surely.
* Conditions for acceptance This is not because the library is not relevant or useful, or because of bad design, but because it misses important functionality in the current state that would give it impact in the current ecosystem. "If I already have to use two competing point libraries, why should I additionally introduce qvm?" The scope must be broadened by including some advanced algorithms which make qvm useful in the ecosystem, also interoperability with already existing boost components must be established. Some of its functionality already exists in boost, which makes acceptance as a standalone library a bit odd. It could be worthwhile to merge qvm with another geometry related boost library to strengthen the links between the libraries.
Thanks! Do you vote for conditional acceptance under the condition that in addition to basic operations also more advanced algorithms should be implemented in the future? Or that the library shouldn't be accepted at this point? Emil are you willing to extend the scope of the library? I'm guessing that if Emil wanted to implement some advanced algorithms from some specific domain he'd be forced to redesign parts of the library, e.g. represent data in a different way, provide specialized containers, etc. It's also possible that this approach would be incompatible with some other one suitable for a different domain. And this is the reason why QVM is implemented on a high level of abstraction. So it may be used as a link between various domain-specific libraries. Is that correct? Adam
Oswin Krause wrote:
We encourage your participation in this review. At a minimum, kindly
state: - Whether you believe the library should be accepted into Boost
Not now, but at a later point surely.
* Conditions for acceptance
This is not because the library is not relevant or useful, or because of bad design, but because it misses important functionality in the current state that would give it impact in the current ecosystem. "If I already have to use two competing point libraries, why should I additionally introduce qvm?" The scope must be broadened by including some advanced algorithms which make qvm useful in the ecosystem, also interoperability with already existing boost components must be established. Some of its functionality already exists in boost, which makes acceptance as a standalone library a bit odd. It could be worthwhile to merge qvm with another geometry related boost library to strengthen the links between the libraries.
Thanks!
Do you vote for conditional acceptance under the condition that in addition to basic operations also more advanced algorithms should be implemented in the future? Or that the library shouldn't be accepted at this point? Emil are you willing to extend the scope of the library?
Hi Adam, I believe that since this library specifically targets operations in geometric spaces in 2/3/4d, advanced operations are out of the scope for this library. It is my understanding (and I may be very wrong here since Oswin is a much more senior member of this community than me) that QVM supports all the operations that I would currently expect from it - it is not a substitute for uBLAS since uBLAS is a complete linear algbera library with solvers and advanced matrix functionalities. QVM caters to the graphics community with support for operations like swizzling which I often use when I am working with GLSL shaders(the client ops.). Just like I use glm and eigen in my current projects, I can envision people using QVM and uBLAS in a similar fashion with one complementing the other. Also from reading the clarifications to my review, I understand that most of my concerns were alleviated and *hence I would recommend that this library be added to boost. *
I'm guessing that if Emil wanted to implement some advanced algorithms from some specific domain he'd be forced to redesign parts of the library, e.g. represent data in a different way, provide specialized containers, etc. It's also possible that this approach would be incompatible with some other one suitable for a different domain. And this is the reason why QVM is implemented on a high level of abstraction. So it may be used as a link between various domain-specific libraries. Is that correct?
Adam
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Sun, Dec 13, 2015 at 3:23 PM, Rajaditya Mukherjee < rajaditya.mukherjee@gmail.com> wrote:
Oswin Krause wrote:
We encourage your participation in this review. At a minimum, kindly
state: - Whether you believe the library should be accepted into Boost
Not now, but at a later point surely.
* Conditions for acceptance
This is not because the library is not relevant or useful, or because of bad design, but because it misses important functionality in the current state that would give it impact in the current ecosystem. "If I already have to use two competing point libraries, why should I additionally introduce qvm?" The scope must be broadened by including some advanced algorithms which make qvm useful in the ecosystem, also interoperability with already existing boost components must be established. Some of its functionality already exists in boost, which makes acceptance as a standalone library
a
bit odd. It could be worthwhile to merge qvm with another geometry related boost library to strengthen the links between the libraries.
Thanks!
Do you vote for conditional acceptance under the condition that in addition to basic operations also more advanced algorithms should be implemented in the future? Or that the library shouldn't be accepted at this point? Emil are you willing to extend the scope of the library?
Hi Adam, I believe that since this library specifically targets operations in geometric spaces in 2/3/4d, advanced operations are out of the scope for this library. It is my understanding (and I may be very wrong here since Oswin is a much more senior member of this community than me) that QVM supports all the operations that I would currently expect from it - it is not a substitute for uBLAS since uBLAS is a complete linear algbera library with solvers and advanced matrix functionalities. QVM caters to the graphics community with support for operations like swizzling which I often use when I am working with GLSL shaders(the client ops.). Just like I use glm and eigen in my current projects, I can envision people using QVM and uBLAS in a similar fashion with one complementing the other.
Rajaditya, I really appreciate this comment, you're absolutely right about the scope of QVM, this is not a generic linear algebra library, though it also isn't "yet another 3D graphics math library" either -- its more appropriate to think of it as a meeting place for the many other such libraries that exist already, since medium- and large-scale programs tend to use multiple Q, V and M types coming from several different APIs. That said, and to reply to Adam's question, I believe that Oswin does have a point, it may be appropriate to broaden the scope to include functionality that's used in 3D applications other than graphics. Emil
Hi all, On 2015-12-14 00:23, Rajaditya Mukherjee wrote:
Oswin Krause wrote:
Do you vote for conditional acceptance under the condition that in addition to basic operations also more advanced algorithms should be implemented in the future? Or that the library shouldn't be accepted at this point? Emil are you willing to extend the scope of the library?
Hi Adam, I believe that since this library specifically targets operations in geometric spaces in 2/3/4d, advanced operations are out of the scope for this library. It is my understanding (and I may be very wrong here since Oswin is a much more senior member of this community than me) that QVM supports all the operations that I would currently expect from it - it is not a substitute for uBLAS since uBLAS is a complete linear algbera library with solvers and advanced matrix functionalities. QVM caters to the graphics community with support for operations like swizzling which I often use when I am working with GLSL shaders(the client ops.). Just like I use glm and eigen in my current projects, I can envision people using QVM and uBLAS in a similar fashion with one complementing the other.
There are many specialized use-cases for 2,3 and 4D geometry which are not in the scope of the BLAS packages - but important for the graphics people and similar fields. I am not talking about big algorithms for every use-case like a general SVD. But the library should offer more than the bare minimum of operations required for 3D Geometry. It does not touch most of basic operations for points (e.g. clamping) or projective geometry. Similarly, it misses most stuff needed for kinematics - i can define a rotation, but can not easily figure out which force it applies(the FAQ section explicitly mentions "simulations", therefore I assumed some basic physics to be relevant). Of course, one can construct most of this using the basic operations in the library, but it would be nice if it had it in the first place, especially as it requires some effort and enable_if magic to add new operations I am not sure whether a pure "glue-code" library is going to be successful as part of boost. I see the need for a library that can handle whatever is thrown at it, but this also requires that it offers a wide bandwidth of operations for many use-cases and/or an interface that is easy to extend. Regarding the latter: the documentation currently falls short on describing how to extend the library and therefore I conclude that this is not a goal.
Do you vote for conditional acceptance under the condition that in addition to basic operations also more advanced algorithms should be implemented in the future?
I am going to vote for conditional acceptance under the condition of a consensus on the scope of the library(i.e. I am not going to stand in the way if the boost community sees it as sufficient if QVM is a pure glue between more powerful libraries). If the scope is intended to be broader than "glue code" I would like to hear about a roadmap for what is going to be added or how the library author envisions its development. Otherwise the library looks pretty okay, aside of a few naming issues which I do not consider to be a problem as they are easy fixes (transp() and trans() are misleading, then there are many shorthands like deduce_s which are not very obvious to understand, the mag() issue raised earlier etc.). Best, Oswin
On Mon, Dec 14, 2015 at 7:17 AM, Oswin Krause < Oswin.Krause@ruhr-uni-bochum.de> wrote:
I am going to vote for conditional acceptance under the condition of a consensus on the scope of the library(i.e. I am not going to stand in the way if the boost community sees it as sufficient if QVM is a pure glue between more powerful libraries). If the scope is intended to be broader than "glue code" I would like to hear about a roadmap for what is going to be added or how the library author envisions its development.
In general yours and the comments of others about extending the scope of QVM seem reasonable, and there seems to be a consensus that this is not a general linear algebra library, which addresses my biggest concern about extensions: I don't want the library to lose focus or to become less lean. Do you think you could come up with a formal list of additional operations that you think should be part of the library? Thanks, Emil
participants (6)
-
Adam Wulkiewicz -
Agustín K-ballo Bergé -
Emil Dotchevski -
Emil Dotchevski -
Oswin Krause -
Rajaditya Mukherjee