Update to use of Eigen and Address transformation problem w/ TR cards #66

pshriwise · 2019-08-30T18:49:46Z

Summary

This PR intends to remove the armadillo dependency from the project and replaces it with eigen (hopefully resolving #51) and correct lattice transformations (not only translations like in #41) originally reported by @YeChengKSU.

Other notable changes:

adds a CMake test related to the characterization of GQ surfaces using the following reference form Radford University
adds a few representative GQ tests for both axis-aligned and transformed ellipsoids and elliptic cylinders

My apologies for the relatively large PR, but it was really easiest (and most time-efficient on my side) to combine these efforts into one.

Verification

I've tested all of the files in both /tests/gqs and /tests/nested-lattice-tests with both the master branch and eigen_update using both Trelis and OCE. No changes were observed in the resulting CAD files when using Trelis, but when testing OCE I noticed a few issues:

elliptic cylinders seem malformed
the rotated elliptic cylinder failed to generate using the master branch
curve artifacts were seen when generating lattice files

Poorly formed Elliptic Cylinder

Highlighted Curve Artifact

I'd really appreciate some verification from @zxkjack123 and @YeChengKSU of what I saw when testing the files mentioned above. Someday we'll have automated tests for this maybe...

Finally, thank you to @SamuelStern for the bulk of the leg work in our switch to eigen!

…d lattices.

…sues" This reverts commit 9979b03. Namespace issues do not appear to acutally be at fault

This reverts commit 68ee4f5. Experimenting with and without semicolon to determine source of file-finding issue

This reverts commit f6d69a4. Confirmed that adding the semicolon is somehow causing the file to not be found

…cnp2cad.

…ntation.

…ere made in this file.

GQ_Characterize.cpp

gonuke

Thanks @pshriwise - this looks like it should do the trick!!

I reviewed this mostly from the point of view of whether it is algorithmically the same as the Armadillo version, and found a few questions/differences. Otherwise, I think this looks ready to go!

volumes.hpp

GQ_Characterize.hpp

GQ_Characterize.cpp

README.rst

GQ_Characterize.hpp

Co-Authored-By: Xiaokang Zhang <[email protected]>

gonuke · 2019-09-03T01:21:22Z

I'm ready to merge after updates to the README

gonuke · 2019-09-03T12:38:56Z

Thanks @pshriwise - now to see how this works in Windows!!!

pshriwise and others added 30 commits August 30, 2019 13:36

Adding a function for generation of a full 3D transformation matrix.

1982e48

Adding a function for combining transformations.

3339b7f

Replacing translation only fix with full transformation fix for neste…

b461d52

…d lattices.

Changed volumes.cpp to use Eigen

b17983d

Removed arma inclusion

1f874da

Updated eigen include line

c49bc05

Several bugfixes for Eigen syntax

6b895cc

Patches to numerous syntax and logic errors

94d3589

Fixed last volumes.cpp compiler error

5a962e7

Removed armadillo dependency in CMakeLists

611939a

Update README for current status

604ed03

Add hyperlinks

ea68e74

minor editing and update Authors

1093a49

add space

7b3453e

Add test file

bdb9697

Added GQ_Characterize class to allow testing of type characterization

b5b7087

Separated GQ_Characterize declaration and implementation

386c44d

Modified CMakeLists to include new class

95e88ae

Fixed GQ_char constructor call, added Eigen include

05bbf0e

Removed Eigen Include line (redundant), changed origin declaration

191b890

Removed Eigen Include line (redundant), changed origin declaration

a1ae04c

Changed name of 'origin' to 'origin_pt' to avoid namespace issues

635b1d3

Revert "Changed name of 'origin' to 'origin_pt' to avoid namespace is…

1d2e264

…sues" This reverts commit 9979b03. Namespace issues do not appear to acutally be at fault

Added missing semicolon to GQ_char definition

aef7d64

Revert "Added missing semicolon to GQ_char definition"

b0acb0b

This reverts commit 68ee4f5. Experimenting with and without semicolon to determine source of file-finding issue

Revert "Revert "Added missing semicolon to GQ_char definition""

a29e1ce

This reverts commit f6d69a4. Confirmed that adding the semicolon is somehow causing the file to not be found

Changed inclusion syntax

9690abc

Changed const variable initialization

b05514f

added Eigen:: to relevant variables

775da90

Experiment with resolution of overloaded Vector3d class

2022129

Samuel Stern and others added 15 commits August 30, 2019 13:38

Fixed glitch in counting eigenvalue signs

15dfa40

Fixed unit test inputs

c862c6b

Added non-unit (rotated) tests, commented out unit tests (all pass)

2d63541

Adding internal check in GQ tests. Adding test program as a test of m…

3132c39

…cnp2cad.

Improving failed test output a bit.

2951c72

Correction to coefficients of the ELLIPTIC_CYL test (svalinn#13)

b055d94

Containing the MPINV in its own function and using an updated impleme…

a781b7c

…ntation.

Setting transform on GQ volume class.

ea017f9

Removing some comments. Using the Eigen-native pseudoInverse method.

bee837d

Adding a test case. Using the determinant method of the original matrix.

e0e539c

Cleanup of the GQ characterize class.

53d75bc

Updating build instructions and adding a check for the Eigen3 library.

13d3627

Some test formatting updates.

88762a4

Replacing volumes.hpp with the version from master. No code changes w…

dac3743

…ere made in this file.

Adding GQ test files.

e8a2f39

pshriwise requested review from gonuke and zxkjack123 August 30, 2019 18:49

pshriwise force-pushed the eigen_update branch from 9f0ed21 to e8a2f39 Compare August 30, 2019 18:52

gonuke reviewed Aug 31, 2019

View reviewed changes

GQ_Characterize.cpp Show resolved Hide resolved

gonuke reviewed Sep 1, 2019

View reviewed changes

volumes.hpp Outdated Show resolved Hide resolved

GQ_Characterize.hpp Show resolved Hide resolved

GQ_Characterize.cpp Show resolved Hide resolved

README.rst Show resolved Hide resolved

README.rst Outdated Show resolved Hide resolved

Correction to volumes.hpp whitespace.

2b6fdb4

zxkjack123 reviewed Sep 2, 2019

View reviewed changes

GQ_Characterize.hpp Show resolved Hide resolved

GQ_Characterize.hpp Outdated Show resolved Hide resolved

pshriwise and others added 3 commits September 2, 2019 16:24

Updates to README.

15bbcc0

Updating GQ_Characterize constructor format and adding a comment.

381811b

Update GQ_Characterize.hpp

7e44fdc

Co-Authored-By: Xiaokang Zhang <[email protected]>

Moving a section in the README.

64ffb55

gonuke merged commit b0675d7 into svalinn:master Sep 3, 2019

pshriwise mentioned this pull request Sep 3, 2019

GQ Characterization Tolerances #21

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to use of Eigen and Address transformation problem w/ TR cards #66

Update to use of Eigen and Address transformation problem w/ TR cards #66

pshriwise commented Aug 30, 2019 •

edited

Loading

gonuke left a comment

gonuke commented Sep 3, 2019

gonuke commented Sep 3, 2019

Update to use of Eigen and Address transformation problem w/ TR cards #66

Update to use of Eigen and Address transformation problem w/ TR cards #66

Conversation

pshriwise commented Aug 30, 2019 • edited Loading

Summary

Other notable changes:

Verification

Poorly formed Elliptic Cylinder

Highlighted Curve Artifact

gonuke left a comment

Choose a reason for hiding this comment

gonuke commented Sep 3, 2019

gonuke commented Sep 3, 2019

pshriwise commented Aug 30, 2019 •

edited

Loading