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

Feature/gridedit 777 casulli refinement #283

Merged
merged 26 commits into from
Jan 31, 2024

Conversation

BillSenior
Copy link
Contributor

No description provided.

libs/MeshKernel/include/MeshKernel/CasulliRefinement.hpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
///
/// Returns 0 when the node is at the start of the edge, 1 when it is at the end
/// and the null value when the edge is not connected to the node.
UInt IsStartOrEnd(const UInt edgeId, const UInt nodeId) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

in entities there is a function OtherNodeOfEdge, shall we move OtherNodeOfEdge also in mesh2d class?

/// @brief Find the id of the element that is common to both edges.
///
/// If no such element can be found then the null value will be returned.
UInt FindCommonElement(const UInt edge1, const UInt edge2) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

FindCommonFace, for keeping the implementation consistent

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add doc for the parameters of all methods.

Comment on lines 86 to 89
static void ConnectNewNodes(Mesh2D& mesh, const std::vector<EdgeNodes>& newNodes, const UInt numNodes, const UInt numEdges, const UInt numFaces, std::vector<NodeMask>& nodeMask);

/// @brief Add newly generated nodes to the newNodes list.
static void StoreNewNode(const Mesh2D& mesh, const UInt nodeId, const UInt edge1Index, const UInt edge2Index, const UInt newNodeId, std::vector<EdgeNodes>& newNodes);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have each parameter on a separate line for better readability (Also in the source file).

@@ -160,6 +160,11 @@ namespace meshkernel
/// @return If the face is on boundary
[[nodiscard]] bool IsFaceOnBoundary(UInt face) const;

/// @brief Get the local index of the node belong to a face.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing param doc

void meshkernel::CasulliRefinement::Compute(Mesh2D& mesh)
{
Polygons emptyPolygon;
Compute(mesh, emptyPolygon);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can pass a Polygons constructor here


std::vector<meshkernel::CasulliRefinement::NodeMask> meshkernel::CasulliRefinement::InitialiseNodeMask(const Mesh2D& mesh, const Polygons& polygon)
{
std::vector<NodeMask> nodeMask(10 * mesh.GetNumNodes(), NodeMask::Unassigned);
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm...

Comment on lines +45 to +46
UInt node1 = mesh.m_edges[i].first;
UInt node2 = mesh.m_edges[i].second;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can us e structured binding. Either way please make const.

Comment on lines +75 to +76
UInt elementId = mesh.m_edgesFaces[edge1][0];
UInt nodeCount = mesh.m_numFacesNodes[elementId];
Copy link
Contributor

Choose a reason for hiding this comment

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

const

libs/MeshKernel/src/CasulliRefinement.cpp Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
libs/MeshKernel/src/CasulliRefinement.cpp Outdated Show resolved Hide resolved
@BillSenior BillSenior merged commit 48a02dc into master Jan 31, 2024
11 checks passed
@BillSenior BillSenior deleted the feature/GRIDEDIT-777_casulli_refinement branch January 31, 2024 13:39
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.

3 participants