Skip to content
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 ElementCoordinatesMappingLocal class #655

Merged
merged 22 commits into from
Jun 1, 2015

Conversation

norihiro-w
Copy link
Collaborator

This PR is a part of #595 and adds mapping of lower dimensional elements (e.g. line in 2D, tri in 3D) between global and local coordinates. The mapping functionality will be used during calculation of shape functions and mapping of tensor material properties like permeability.

@norihiro-w
Copy link
Collaborator Author

Remark: one of the tests is not working right now due to an issue in computeRotationMatrixToXY.(). Tom will fix it (#659).

@@ -60,11 +60,28 @@ Orientation getOrientation (const GeoLib::Point* p0,
* @param plane_normal the normal of the plane the polygon is located in
* @param d parameter from the plane equation
*/
void getNewellPlane (const std::vector<GeoLib::Point*>& pnts,
template <class T_POINT>
void getNewellPlane (const std::vector<T_POINT*>& pnts,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the function name it is not clear what is modified and how.
Is it possible to return the normal vector and the distance, instead of modifying values?

Update: never mind the Newell, I didn't know the Newell's method for computing a plane from a polygon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, we should rename the method to something like getSupportingPlane. The algorithm uses Newell's method, described in the book "Real-Time Collision Detection" from Christer Ericson. This fact is also missing in the documentation.
@norihiro-w Should I update the branch or should we make a separate PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can make a separate PR

@norihiro-w
Copy link
Collaborator Author

This PR should be merged after #659 by Tom. I will rebase my commits to it

@norihiro-w
Copy link
Collaborator Author

rebased to #659

*/
struct CoordinateSystemType
{
enum type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use enum class directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i initially tried to use it but changed it to this way because the code is shorter and simpler as I don't need to make cast (from enum class to int) each time whenever bit operation is used.

@TomFischer
Copy link
Member

@norihiro-w Please let me know if I should help you fixing some of the issues.

// rotate the point coordinates
for(unsigned i = 0; i < ele.getNNodes(); i++)
{
Eigen::Vector3d x_new = matR2local * Eigen::Map<Eigen::Vector3d>(const_cast<double*>(vec_pt[i]->getCoords()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an obstacle to implement something like

MeshLib::Node operator*(EigenMatrix const& mat, MeshLib::Node const& node)

or

MathLib::Point3d operator*(EigenMatrix mat, MathLib::Point3d const& )?

I would prefer the second version, which is more general.

This would be
(1) more readable for the user and
(2) save some lines of code and
(3) omit some copies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. please see ee35769

@norihiro-w
Copy link
Collaborator Author

got the following errors with MSVC2013(Express). is there anyone having idea what to do? strangely the error does not appear on Jenkins.

Error   28  error C2280: 'ProcessLib::GroundwaterFlow::LocalAssemblerData<NumLib::ShapeLine2,NumLib::IntegrationGaussRegular<0x01>,GlobalMatrix_,GlobalVector_>::LocalAssemblerData(void)' : attempting to reference a deleted function C:\Users\watanabe\Documents\GitHub\ogs\AssemblerLib\LocalDataInitializer.h  59  1   ogs
Error   35  error C2542: 'NumLibFemIsoTest<Type>::D' : class object has no constructor for initialization   C:\Users\watanabe\Documents\GitHub\ogs\Tests\NumLib\TestFe.cpp  104 1   testrunner
Error   36  error C2542: 'NumLibFemIsoTest<Type>::expectedM' : class object has no constructor for initialization   C:\Users\watanabe\Documents\GitHub\ogs\Tests\NumLib\TestFe.cpp  104 1   testrunner
Error   37  error C2542: 'NumLibFemIsoTest<Type>::expectedK' : class object has no constructor for initialization   C:\Users\watanabe\Documents\GitHub\ogs\Tests\NumLib\TestFe.cpp  104 1   testrunner
Error   38  error C1903: unable to recover from previous error(s); stopping compilation C:\Users\watanabe\Documents\GitHub\ogs\Tests\NumLib\TestFe.cpp  109 1   testrunner

@endJunction
Copy link
Member

I compiled the code with clang in RelWithDebInfo, which run through OK. But running tests ended in a SEGV for the:

[----------] 3 tests from NumLibFemIsoTest/5, where TypeParam = (anonymous namespace)::TestCase<FeTestData::TestFeLINE2, EigenFixedShapeMatrixPolicy>
[ RUN      ] NumLibFemIsoTest/5.CheckMassMatrix

test.
Running it in gdb does not always result in a SEGV: output and the backtrace here

Valgrind's memcheck does not show any invalid reads/writes until a SEGV. (see the above gist for details)

Update: After some through testing I'm not sure if my error report is related to the MSVC compilation error.
When removing the --march=native from the clang's compiler flags, everything is green again.
Clang is version 3.6.1. With gcc 5.1.0 and -march=native there is no error.

@norihiro-w
Copy link
Collaborator Author

update: the test failing on Jenkins cannot be reproduced on my windows (win7, MSVC2013 with update 4, 64bit). could be there is something unitialized or machine dependent bug.

@norihiro-w
Copy link
Collaborator Author

rebased to HEAD

@ogsbot
Copy link
Member

ogsbot commented Jun 1, 2015

Jenkins: OGS-6/Win-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Win-PRs/140/

@ogsbot
Copy link
Member

ogsbot commented Jun 1, 2015

Jenkins: OGS-6/Win-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Win-PRs/141/

@norihiro-w
Copy link
Collaborator Author

@bilke Eigen version used in OGS6/Win-PRs seems to be bit old (3.1.4). Can you please replace it with the latest (3.2.4)?

@ogsbot
Copy link
Member

ogsbot commented Jun 1, 2015

Jenkins: OGS-6/Win-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Win-PRs/142/

@bilke
Copy link
Member

bilke commented Jun 1, 2015

I updated eigen to 3.2.4.

Jenkins test this please!

@bilke
Copy link
Member

bilke commented Jun 1, 2015

test this please

@ogsbot
Copy link
Member

ogsbot commented Jun 1, 2015

Jenkins: OGS-6/Win-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Win-PRs/143/

@norihiro-w
Copy link
Collaborator Author

@bilke thanks!

@norihiro-w
Copy link
Collaborator Author

the problem was solved!

endJunction added a commit that referenced this pull request Jun 1, 2015
Add ElementCoordinatesMappingLocal class
@endJunction endJunction merged commit 4448b55 into ufz:master Jun 1, 2015
@norihiro-w norihiro-w deleted the add-element-mapping-local branch June 2, 2015 07:19
@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants