Feature/gridedit 2102 grading in triangulation#535
Conversation
…on from meshkernel
…ssing to other computer)
This contains a conversion of the SEPRAN Fortran code to c++ using Claude Sonnet. Current it computes a slightly different triangulation that the original fortran code.
|
|
||
| sepran::mshoce( | ||
| newMesh != 0, |
There was a problem hiding this comment.
Change name to something more meaningful
| brew install boost doxygen libaec libomp | ||
| brew install boost doxygen gcc libaec libomp | ||
|
|
||
| - name: Set Fortran compiler (macOS) |
There was a problem hiding this comment.
Can be removed now that there's no Fortran code anymore?
| # Common warning and visibility flags | ||
| add_compile_options("-fvisibility=hidden;-Wall;-Wextra;-pedantic;-Werror;-Wno-unused-function") | ||
|
|
||
| cmake_host_system_information(RESULT os_release QUERY OS_RELEASE) |
There was a problem hiding this comment.
CMake has a (more robust) command for checking compiler flags:
include(CheckCXXCompilerFlag)
check_cxx_compiler_flag("-Wno-character-conversion" SUPPORTS_WNO_CHARACTER_CONVERSION)
if(SUPPORTS_WNO_CHARACTER_CONVERSION)
add_compile_options("-Wno-character-conversion")
endif()| # Optimization / debug flags | ||
| add_compile_options("$<$<CONFIG:RELEASE>:-O2>") | ||
| add_compile_options("$<$<CONFIG:DEBUG>:-g>") | ||
| # Disable warnings about implicit conversions between character types |
There was a problem hiding this comment.
Comment seems out of place.
| virtual ~TriangulationGenerator() = default; | ||
|
|
||
| /// \brief Compute triangulation | ||
| virtual std::unique_ptr<Mesh2D> generate(const Polygons& polygon) const = 0; |
There was a problem hiding this comment.
Shouldn't PascalCase naming style be used (consistent with the rest of the code)?
|
|
||
| for (UInt i = 0; i < NumberOfInner(); ++i) | ||
| { | ||
| auto [innerMinimum, innerMaximum] = Inner(i).SegmentLengthExtrema(); |
There was a problem hiding this comment.
Missing handling for constants::missing::doubleValue from Polygon::SegmentLengthExtrema.
| target_include_directories(${target_name} PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) | ||
|
|
||
| target_compile_features(${target_name} PUBLIC cxx_std_20) | ||
|
|
There was a problem hiding this comment.
Add precompiled headers to speedup compilation like in libs\MeshKernel\CMakeLists.txt?
| POSITION_INDEPENDENT_CODE ON | ||
| ) | ||
|
|
||
| source_group("Source Files" FILES ${TARGET_SRC_LIST}) No newline at end of file |
There was a problem hiding this comment.
Also add header files group.
|
|
||
| private: | ||
| /// \brief The scale factor used when generating points in polygon | ||
| const double scaleFactor_; |
There was a problem hiding this comment.
Underscore should be dropped from the name (naming convention).
|
|
||
| private: | ||
| /// \brief Find the smallest delta in the boundary polygon | ||
| static double minimumEdgeDelta(const std::vector<meshkernel::Point>& polygonNodes); |
There was a problem hiding this comment.
Unused; can be removed?
| // Compute the minimum spacing between points of the polygon, both outer and all inner polygons | ||
| const auto [minimumDelta, maximumDeltaDummy] = polygon.Enclosure(0).SegmentLengthExtrema(); | ||
|
|
||
| const int estimatedNumberOfElements = static_cast<int>((estimatedArea / (0.433 * minimumDelta * minimumDelta)) * 3.5); |
There was a problem hiding this comment.
What are these magic numbers (0.433, 3.5)? Consider adding named constants for readability.
| // Get estimate of area covered by polygon, subtracting area covered by holes | ||
| const double estimatedArea = polygon.Enclosure(0).ComputeSurfaceArea(); | ||
| // Compute the minimum spacing between points of the polygon, both outer and all inner polygons | ||
| const auto [minimumDelta, maximumDeltaDummy] = polygon.Enclosure(0).SegmentLengthExtrema(); |
There was a problem hiding this comment.
Should a constants::missing::doubleValue return be handled here?
No description provided.