From 5c5b03e85c06b74c693ded63a484509f155ac1c7 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Sun, 20 Aug 2023 22:30:49 +0530 Subject: [PATCH 1/8] remove static lib target and rename `firefly_shared` target to `firefly` --- CMakeLists.txt | 16 ++++++++-------- README.md | 6 +----- examples/CMakeLists.txt | 2 +- src/vector/CMakeLists.txt | 5 ++--- tests/CMakeLists.txt | 2 +- 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 005a742..7eced88 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,14 +16,14 @@ endif() include_directories(headers) -add_library(${PROJECT_NAME}_static STATIC) -add_library(${PROJECT_NAME}_shared SHARED) -set_target_properties(${PROJECT_NAME}_static PROPERTIES OUTPUT_NAME ${PROJECT_NAME}) -set_target_properties(${PROJECT_NAME}_shared PROPERTIES OUTPUT_NAME ${PROJECT_NAME}) +add_library(${PROJECT_NAME} SHARED) -add_library(Firefly::Shared ALIAS ${PROJECT_NAME}_shared) -add_library(Firefly::Static ALIAS ${PROJECT_NAME}_static) +if(MSVC) + target_compile_options(${PROJECT_NAME} PRIVATE /W4 /WX) +else() + target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wpedantic -Werror) +endif() add_subdirectory(src) @@ -50,5 +50,5 @@ if (${Firefly_ENABLE_TESTS}) add_subdirectory(tests) endif() -install(TARGETS ${PROJECT_NAME}_static ${PROJECT_NAME}_shared) -install(DIRECTORY headers/ DESTINATION include) +install(TARGETS ${PROJECT_NAME}) +install(DIRECTORY headers DESTINATION include) diff --git a/README.md b/README.md index a01dbce..ee8d664 100644 --- a/README.md +++ b/README.md @@ -99,11 +99,7 @@ g++ main.cpp -DDOUBLE_PRECISION=1 -lfirefly -o mycode ### Build using `CMake` ```cmake -# for shared linking -target_link_libraries(${PROJECT_NAME} PUBLIC Firefly::Shared) - -# for static linking -target_link_libraries(${PROJECT_NAME} PUBLIC Firefly::Static) +target_link_libraries(${PROJECT_NAME} PUBLIC firefly) ``` ## Future Plans diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 8ecce5e..bb3a752 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -6,4 +6,4 @@ set(CXX_STANDARD 17) add_executable(${PROJECT_NAME} main.cpp) -target_link_libraries(${PROJECT_NAME} Firefly::Shared) \ No newline at end of file +target_link_libraries(${PROJECT_NAME} firefly) \ No newline at end of file diff --git a/src/vector/CMakeLists.txt b/src/vector/CMakeLists.txt index 8760bda..e877afc 100644 --- a/src/vector/CMakeLists.txt +++ b/src/vector/CMakeLists.txt @@ -1,5 +1,4 @@ -file(GLOB SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/*.cpp) +file(GLOB SOURCES *.cpp) -target_sources(${PROJECT_NAME}_shared PRIVATE ${SOURCES}) -target_sources(${PROJECT_NAME}_static PRIVATE ${SOURCES}) \ No newline at end of file +target_sources(${PROJECT_NAME} PRIVATE ${SOURCES}) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ad1da36..70acebc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,6 +1,6 @@ file(GLOB_RECURSE SOURCES "*.cpp") add_executable(FireflyTests ${SOURCES}) -target_link_libraries(FireflyTests PRIVATE GTest::gtest_main Firefly::Shared) +target_link_libraries(FireflyTests PRIVATE GTest::gtest_main firefly) gtest_discover_tests(FireflyTests) \ No newline at end of file From 9a02db85477ed8e746227158d9b43aae85662ba0 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Sun, 20 Aug 2023 22:51:51 +0530 Subject: [PATCH 2/8] fix `-Wsign-compare` in is_sparse method --- src/vector/is_sparse.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vector/is_sparse.cpp b/src/vector/is_sparse.cpp index f825c37..fd72106 100644 --- a/src/vector/is_sparse.cpp +++ b/src/vector/is_sparse.cpp @@ -1,4 +1,5 @@ #include +#include #include #include "firefly/vector.hpp" @@ -9,8 +10,8 @@ bool Vector::IsSparse() const { throw std::length_error("Can't determine sparseness of empty vector"); } - auto const zero_count = std::count_if(m_vec.cbegin(), m_vec.cend(), - [](Real const &v) { return v == 0; }); + size_t const zero_count = std::count_if(m_vec.cbegin(), m_vec.cend(), + [](Real const &v) { return v == 0; }); return (m_vec.size() - zero_count) < zero_count; } From 8ed80751529e69dde0205dae474b7ec5f33778e4 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Sun, 20 Aug 2023 22:59:02 +0530 Subject: [PATCH 3/8] implement ON/OFF matrix of `Firefly_ENABLE_DOUBLE_PRECISION` --- .github/workflows/unit_testing.yaml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/unit_testing.yaml b/.github/workflows/unit_testing.yaml index 59f330f..1b2366c 100644 --- a/.github/workflows/unit_testing.yaml +++ b/.github/workflows/unit_testing.yaml @@ -18,11 +18,12 @@ jobs: name: Run test generator script unit_test_gcc_linux: - name: Linux (GCC=v${{ matrix.version }}) + name: Linux (GCC=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) needs: [ ensure_tests ] strategy: matrix: version: [9, 10, 11, 12, 13] + double-precision: [ON, OFF] runs-on: ubuntu-latest env: CC: gcc-${{ matrix.version }} @@ -35,7 +36,7 @@ jobs: with: version: ${{ matrix.version }} - run: | - cmake -Bbuild -DFirefly_ENABLE_TESTS=ON + cmake -Bbuild -DFirefly_ENABLE_TESTS=ON -DFirefly_ENABLE_DOUBLE_PRECISION=${{ matrix.double-precision }} -DFirefly_ENABLE_EXAMPLES=ON cmake --build build name: Configure and build binaries - run: | @@ -43,11 +44,12 @@ jobs: name: Run tests unit_test_llvm_linux: - name: Linux (LLVM=v${{ matrix.version }}) + name: Linux (LLVM=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) needs: [ ensure_tests ] strategy: matrix: version: [11, 12, 13, 14, 15, 16] + double-precision: [ON, OFF] runs-on: ubuntu-latest env: CC: clang-${{ matrix.version }} @@ -60,7 +62,7 @@ jobs: with: version: ${{ matrix.version }} - run: | - cmake -Bbuild -DFirefly_ENABLE_TESTS=ON + cmake -Bbuild -DFirefly_ENABLE_TESTS=ON -DFirefly_ENABLE_DOUBLE_PRECISION=${{ matrix.double-precision }} -DFirefly_ENABLE_EXAMPLES=ON cmake --build build name: Configure and build binaries - run: | @@ -68,11 +70,12 @@ jobs: name: Run tests unit_test_gcc_macos: - name: MacOS (GCC=v${{ matrix.version }}) + name: MacOS (GCC=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) needs: [ ensure_tests ] strategy: matrix: version: [10, 11, 12, 13] + double-precision: [ON, OFF] runs-on: macos-latest env: CC: gcc-${{ matrix.version }} @@ -84,7 +87,7 @@ jobs: brew install gcc@${{ matrix.version }} name: Install GCC - run: | - cmake -Bbuild -DFirefly_ENABLE_TESTS=ON + cmake -Bbuild -DFirefly_ENABLE_TESTS=ON -DFirefly_ENABLE_DOUBLE_PRECISION=${{ matrix.double-precision }} -DFirefly_ENABLE_EXAMPLES=ON cmake --build build name: Configure and build binaries - run: | @@ -92,11 +95,12 @@ jobs: name: Run tests unit_test_llvm_macos: - name: MacOS (LLVM=v${{ matrix.version }}) + name: MacOS (LLVM=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) needs: [ ensure_tests ] strategy: matrix: version: [12, 13, 14, 15, 16] + double-precision: [ON, OFF] runs-on: macos-latest env: CC: /usr/local/opt/llvm@${{ matrix.version }}/bin/clang @@ -108,7 +112,7 @@ jobs: brew install llvm@${{ matrix.version }} name: Install LLVM - run: | - cmake -Bbuild -DFirefly_ENABLE_TESTS=ON + cmake -Bbuild -DFirefly_ENABLE_TESTS=ON -DFirefly_ENABLE_DOUBLE_PRECISION=${{ matrix.double-precision }} -DFirefly_ENABLE_EXAMPLES=ON cmake --build build name: Configure and build binaries - run: | From 8427a377f0455d31cba67c016bc2d16ca0b76724 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Mon, 21 Aug 2023 00:56:17 +0530 Subject: [PATCH 4/8] remove `iostream` header --- src/vector/is_sparse.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vector/is_sparse.cpp b/src/vector/is_sparse.cpp index fd72106..6e0ff26 100644 --- a/src/vector/is_sparse.cpp +++ b/src/vector/is_sparse.cpp @@ -1,5 +1,4 @@ #include -#include #include #include "firefly/vector.hpp" From f73b152018b94e7e49d0f1ee47375e0ab8c0c9a5 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Mon, 21 Aug 2023 01:27:56 +0530 Subject: [PATCH 5/8] simplify `add__scalar` assertions --- tests/vector/add.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/vector/add.cpp b/tests/vector/add.cpp index 8d64bd5..52bf889 100644 --- a/tests/vector/add.cpp +++ b/tests/vector/add.cpp @@ -31,9 +31,6 @@ TEST(Vector, add__scalar) { auto v2 = v1 + 10; ASSERT_EQ(v2.Size(), v1.Size()); - ASSERT_EQ(v2.At(0), 11); - ASSERT_EQ(v2.At(1), 12); - ASSERT_EQ(v2.At(2), 13); - ASSERT_EQ(v2.At(3), 14); + ASSERT_EQ(v2.ElemSum(), 40 + v1.ElemSum()); }); } From 4656e9e51d8d53d69cd60cc770828368abfe0d2c Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Mon, 21 Aug 2023 01:29:27 +0530 Subject: [PATCH 6/8] fix floating-point approximation errors with `ASSERT_NEAR` --- README.md | 3 ++- tests/CMakeLists.txt | 1 + tests/vector/angle_with.cpp | 8 +++----- tests/vector/anti_parallel.cpp | 2 +- tests/vector/scale.cpp | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index ee8d664..1f30d39 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ cmake --build build ctest --test-dir build/tests --verbose ``` +> **Note** If you are using `ASSERT_NEAR` in your test cases, I advice using `Firefly_TEST_EPSILON` macro defined [here](tests/CMakeLists.txt#L5). ## Example Usage @@ -108,6 +109,6 @@ target_link_libraries(${PROJECT_NAME} PUBLIC firefly) ## Contact -Email: tbhaxor@proton.me
+Email: tbhaxor _at_ proton _dot_ me
Twitter: @tbhaxor
LinkedIn: @tbhaxor diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 70acebc..de13f3f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,5 +2,6 @@ file(GLOB_RECURSE SOURCES "*.cpp") add_executable(FireflyTests ${SOURCES}) target_link_libraries(FireflyTests PRIVATE GTest::gtest_main firefly) +target_compile_definitions(FireflyTests PRIVATE -DFirefly_TEST_EPSILON=1e-3) gtest_discover_tests(FireflyTests) \ No newline at end of file diff --git a/tests/vector/angle_with.cpp b/tests/vector/angle_with.cpp index 51083c0..174479c 100644 --- a/tests/vector/angle_with.cpp +++ b/tests/vector/angle_with.cpp @@ -12,16 +12,14 @@ TEST(Vector, angle_with__perp_vector__90) { Firefly::Vector v1{{3, 4}}; Firefly::Vector v2{{-4, 3}}; - ASSERT_NO_THROW({ ASSERT_EQ(v1.AngleWith(v2), M_PI / 2); }); + ASSERT_NO_THROW( + { ASSERT_NEAR(v1.AngleWith(v2), M_PI_2, Firefly_TEST_EPSILON); }); } TEST(Vector, angle_with__parallel_vector__180) { Firefly::Vector v{{1, 3}}; - ASSERT_NO_THROW({ - EXPECT_NEAR(v.AngleWith(v), 0, - std::max(static_cast(0), v.AngleWith(v))); - }); + ASSERT_NO_THROW({ EXPECT_NEAR(v.AngleWith(v), 0, Firefly_TEST_EPSILON); }); } TEST(Vector, angle_with__opp_vector__180) { diff --git a/tests/vector/anti_parallel.cpp b/tests/vector/anti_parallel.cpp index 93747de..dbdc100 100644 --- a/tests/vector/anti_parallel.cpp +++ b/tests/vector/anti_parallel.cpp @@ -9,6 +9,6 @@ TEST(Vector, subtract__unary__make_opposite) { ASSERT_NO_THROW({ ASSERT_TRUE(v.IsParallel(-v)); - ASSERT_EQ(v.AngleWith(-v), M_PI); + ASSERT_NEAR(v.AngleWith(-v), M_PI, Firefly_TEST_EPSILON); }); } diff --git a/tests/vector/scale.cpp b/tests/vector/scale.cpp index 3b554e2..a5fa15d 100644 --- a/tests/vector/scale.cpp +++ b/tests/vector/scale.cpp @@ -28,7 +28,7 @@ TEST(Vector, scale__positive) { TEST(Vector, scale__positive_n__n_times_magnitude) { Firefly::Vector v{{1, 2, 3}}; - ASSERT_EQ((v * 10).Magnitude(), v.Magnitude() * 10); + ASSERT_NEAR((v * 10).Magnitude(), v.Magnitude() * 10, Firefly_TEST_EPSILON); } TEST(Vector, scale__negative__opposite) { @@ -36,6 +36,6 @@ TEST(Vector, scale__negative__opposite) { Firefly::Vector v2 = v1 * -1; ASSERT_EQ(v2, -v1); - ASSERT_EQ(v1.AngleWith(v2), M_PI); + ASSERT_NEAR(v1.AngleWith(v2), M_PI, Firefly_TEST_EPSILON); ASSERT_TRUE(v1.IsParallel(v2)); } \ No newline at end of file From 88c3ae3102c5e8605497c47a0c9ecfda1b9ee095 Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Mon, 21 Aug 2023 01:30:47 +0530 Subject: [PATCH 7/8] remove `-Wpedantic` from compile options --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7eced88..11bbd27 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,7 +22,7 @@ add_library(${PROJECT_NAME} SHARED) if(MSVC) target_compile_options(${PROJECT_NAME} PRIVATE /W4 /WX) else() - target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wpedantic -Werror) + target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Werror) endif() add_subdirectory(src) From 50f3fa558e10b16221159a2f11e4942daacdf0cd Mon Sep 17 00:00:00 2001 From: Gurkirat Singh Date: Mon, 21 Aug 2023 01:37:32 +0530 Subject: [PATCH 8/8] add replace `--verbose` with `--output-on-failure` and run ctest in build/tests workdir --- .github/workflows/unit_testing.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/unit_testing.yaml b/.github/workflows/unit_testing.yaml index 1b2366c..500b4a6 100644 --- a/.github/workflows/unit_testing.yaml +++ b/.github/workflows/unit_testing.yaml @@ -40,8 +40,9 @@ jobs: cmake --build build name: Configure and build binaries - run: | - ctest --test-dir build/tests/ --verbose + ctest --output-on-failure name: Run tests + working-directory: build/tests unit_test_llvm_linux: name: Linux (LLVM=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) @@ -66,8 +67,9 @@ jobs: cmake --build build name: Configure and build binaries - run: | - ctest --test-dir build/tests/ --verbose + ctest --output-on-failure name: Run tests + working-directory: build/tests unit_test_gcc_macos: name: MacOS (GCC=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) @@ -91,8 +93,9 @@ jobs: cmake --build build name: Configure and build binaries - run: | - ctest --test-dir build/tests/ --verbose + ctest --output-on-failure name: Run tests + working-directory: build/tests unit_test_llvm_macos: name: MacOS (LLVM=v${{ matrix.version }}, Double=${{ matrix.double-precision }}) @@ -116,5 +119,6 @@ jobs: cmake --build build name: Configure and build binaries - run: | - ctest --test-dir build/tests/ --verbose + ctest --output-on-failure name: Run tests + working-directory: build/tests