-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add test for spherical_coslat_differential #123
Conversation
…ns, update io.hpp to print differential systems
DimensionCount, | ||
CoordinateSystem | ||
> | ||
get_point() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function to return current differential of this object. By doing this we are able to calculate cross of differential objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at #123 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should submit different PRs for different contexts. Like in this case, separate PRs should be there for unit_vector, magnitude, tests.
Also, you should push after squashing the commits.
auto cross | ||
( | ||
cartesian_differential<Args1...> const& representation1, | ||
Representation2<Args2...> const& representation2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you have made this function to support differentials, you should name the arguments differential1 and differential2 instead of representation1 and representation2 to maintain uniformity in the library. And change this whole function accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I'll do that.
DimensionCount, | ||
CoordinateSystem | ||
> | ||
get_point() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the need of this function? get_differential
is already there in base_differential.hpp
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By doing this we can extend all arithmetic function to work for differentials as well. Otherwise, we may get a conflict of .get_point() in all arithmetic functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this will help in magnitude
function. But functions like sum
, mean
, difference
, dot
use make_cartesian_representation()
and hence can't be extended to work for differentials by defining get_point
for differential
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll also help to calculate cross for two differential classes or one representation class and one differential class. In the code where we used
bg::transform(cartesian1.get_point(), tempPoint1); bg::transform(cartesian2.get_point(), tempPoint2);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sarthak2007 I'm closing this PR. I have created a separate PR #127 for the extension of arithmetic functions to support differential systems.
Ok. I'll create a new PR for every context. |
Description
6.Now, cross can also calculate cross product of differential motion objects too.
References
Tasklist