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

Tool ExtractEntireSurface #2764

Merged
merged 25 commits into from Jan 24, 2020
Merged

Conversation

@TomFischer
Copy link
Member

TomFischer commented Jan 17, 2020

Documentation preview

  1. Feature description was added to the changelog
  2. Tests covering your feature were added?
  3. Any new feature or behavior change was documented?
@TomFischer TomFischer added the WIP 🏗 label Jan 17, 2020
@TomFischer TomFischer force-pushed the TomFischer:ToolExtractEntireSurface branch 2 times, most recently from 55afc3d to 022ef6e Jan 17, 2020
@TomFischer TomFischer added please review and removed WIP 🏗 labels Jan 17, 2020
ProcessLib/HT/Tests.cmake Outdated Show resolved Hide resolved
@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Jan 18, 2020

...especially in the tool documentation toc it reads:

  • Extract Entire Surface
  • Extract Surface

suggesting, that the second tool does extract only some parts of a surface.

@TomFischer TomFischer added WIP 🏗 and removed please review labels Jan 20, 2020
@TomFischer TomFischer force-pushed the TomFischer:ToolExtractEntireSurface branch 4 times, most recently from e3ec3ef to a134393 Jan 20, 2020
@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Jan 21, 2020

Looks better! Few new warnings appeared though.

@TomFischer TomFischer force-pushed the TomFischer:ToolExtractEntireSurface branch from a134393 to fc95de8 Jan 21, 2020
@TomFischer

This comment has been minimized.

Copy link
Member Author

TomFischer commented Jan 21, 2020

Yes, I saw the warnings. Do you have any idea how to fix them?

@TomFischer TomFischer removed the WIP 🏗 label Jan 21, 2020
@TomFischer TomFischer force-pushed the TomFischer:ToolExtractEntireSurface branch 2 times, most recently from 095d9e8 to 16b9291 Jan 22, 2020
@TomFischer

This comment has been minimized.

Copy link
Member Author

TomFischer commented Jan 22, 2020

Fixed the warnings.

@endJunction endJunction force-pushed the TomFischer:ToolExtractEntireSurface branch from 16b9291 to 7e499ce Jan 22, 2020
TomFischer and others added 22 commits Jan 16, 2020
- createNodesFromElements returns only the nodes vector
- createNodesAndIDMapFromElements returns the nodes vector and an id map
- both functions use also the newly implemented function createTemporaryNodesFromElements
The previous impl. uses only the geometric element type. This didn't allow to correctly copy higher order elements.
Make clear that boundary (2d/3d) is handled.
Use the existing functionality.
@TomFischer

This comment has been minimized.

Copy link
Member Author

TomFischer commented Jan 24, 2020

Some import new features implemented in the algorithm is:

  • multi-component properties are also extracted
  • higher order boundary elements are created from higher order bulk elements
@TomFischer TomFischer force-pushed the TomFischer:ToolExtractEntireSurface branch from d181da2 to 78f60b7 Jan 24, 2020
@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Jan 24, 2020

Other reviews?
Looks good. I especially like that there is a single place for handling the element types. 👍

@TomFischer

This comment has been minimized.

Copy link
Member Author

TomFischer commented Jan 24, 2020

@endJunction Thanks for reviewing. I think the tool is now much more general (support for higher order elements and multi-component properties output).

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Jan 24, 2020

@TomFischer Will merge tonight, if there is no other review coming.

@endJunction endJunction merged commit 141c99c into ufz:master Jan 24, 2020
3 checks passed
3 checks passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
ufz.ogs #20200124.4 succeeded
Details
@TomFischer TomFischer deleted the TomFischer:ToolExtractEntireSurface branch Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.