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

migrate tests to Catch2v3 #271

Merged
merged 13 commits into from
Apr 29, 2024
Merged

migrate tests to Catch2v3 #271

merged 13 commits into from
Apr 29, 2024

Conversation

s-sajid-ali
Copy link
Member

No description provided.

@s-sajid-ali s-sajid-ali marked this pull request as draft April 26, 2024 21:47
@s-sajid-ali s-sajid-ali force-pushed the catch2_v3 branch 3 times, most recently from d49fe8b to d2c3241 Compare April 26, 2024 23:06
	modified:   CMakeLists.txt
	new file:   cmake_log
	modified:   src/synergia/bunch/CMakeLists.txt
	modified:   src/synergia/collective/CMakeLists.txt
	modified:   src/synergia/foundation/CMakeLists.txt
	modified:   src/synergia/lattice/CMakeLists.txt
	modified:   src/synergia/libFF/CMakeLists.txt
	modified:   src/synergia/simulation/CMakeLists.txt
	modified:   src/synergia/utils/CMakeLists.txt
	deleted:    src/synergia/utils/catch.hpp
	modified:   src/synergia/utils/catch_test_main.cc
	modified:   src/synergia/utils/tests/CMakeLists.txt
	modified:   src/synergia/utils/tests/test_command_line_arg.cc
	modified:   src/synergia/utils/tests/test_commxx_mpi.cc
	modified:   src/synergia/utils/tests/test_commxx_serdes.cc
	modified:   src/synergia/utils/tests/test_distributed_fft2d.cc
	modified:   src/synergia/utils/tests/test_distributed_fft3d.cc
	modified:   src/synergia/foundation/CMakeLists.txt
	modified:   src/synergia/foundation/tests/CMakeLists.txt
	modified:   src/synergia/foundation/tests/test_four_momentum.cc
	modified:   src/synergia/foundation/tests/test_reference_particle.cc
	modified:   src/synergia/foundation/tests/test_trigon.cc
	modified:   .github/workflows/macos-clang.yml
	modified:   .github/workflows/macos-gcc.yml
@s-sajid-ali
Copy link
Member Author

lattice/tests/test_mx_expr.cc : TEST_CASE("mx_expr_writer") increased tolerance for

  • CHECK(std::stod(s) == Approx(d).margin(1e-16));
  • REQUIRE_THAT(std::stod(s), Catch::Matchers::WithinAbs(d, 1e-14));

@s-sajid-ali
Copy link
Member Author

libFF/tests/test_madx_elements.cc: increased tolerances:

 231 ~ │         REQUIRE_THAT(parts(p, 0),
 232 ~ │                      Catch::Matchers::WithinAbs(madx(p, 0), tolerance * 1e4));
 233 ~ │         REQUIRE_THAT(parts(p, 1),
 234 ~ │                      Catch::Matchers::WithinAbs(madx(p, 1), tolerance * 1e4));
 235 ~ │         REQUIRE_THAT(parts(p, 2),
 236 ~ │                      Catch::Matchers::WithinAbs(madx(p, 2), tolerance * 1e4));
 237 ~ │         REQUIRE_THAT(parts(p, 3),
 238 ~ │                      Catch::Matchers::WithinAbs(madx(p, 3), tolerance * 1e4));
 239 ~ │         REQUIRE_THAT(parts(p, 4),
 240 ~ │                      Catch::Matchers::WithinAbs(-madx(p, 4), tolerance * 1e6));

@s-sajid-ali
Copy link
Member Author

src/synergia/simulation/tests/test_get_tunes3.cc: changed tolerances and targets for get_chromaticities:

 158 ~ │     REQUIRE_THAT(chroms.horizontal_chromaticity,
 159 ~ │                  Catch::Matchers::WithinRel(
 160 ~ │                      -44.380743697521531 * beta,
 161 ~ │                      0.05)); // comparing chromaticities vs. MAD-X good to 5%
 162 ~ │     REQUIRE_THAT(chroms.vertical_chromaticity,
 163 ~ │                  Catch::Matchers::WithinRel(-25.5284944071458533 * beta, 0.05));

@s-sajid-ali
Copy link
Member Author

Running only the drift test for src/synergia/libFF/tests/test_madx_elements.cc on devel3, the output of the following is

synergia/libFF/tests on  devel3 [!] via 🅢 synergia-debug 
❯ ./test_madx_elements  | save -f out_drift.txt

is here : out_drift.txt.

We see the following:

Propagator: turn 1/inf., time = 0.000s, macroparticles = (16) / ()
Propagator: maximum number of turns reached
Propagator: total time = 0.000s
libFF	MadX	margin
1.0448575924245564e-02	1.0448575917959040e-02	6.2865233851905344e-12
1.0000000000000000e-02	1.0000000000000000e-02	0.0000000000000000e+00
1.4172863886368342e-02	1.4172863876938561e-02	9.4297816083388497e-12
1.4999999999999999e-02	1.4999999999999999e-02	0.0000000000000000e+00
-2.7798721156533146e-02	-2.7798722751389171e-02	1.5948560250222954e-09
5.8514076963603889e-02	5.8514076963603889e-02	0.0000000000000000e+00
...

where the margins clearly exceed the tolerance of 1e-14 yet the tests pass!

@s-sajid-ali
Copy link
Member Author

s-sajid-ali commented Apr 29, 2024

The test passes because Catch2v2.13.9 comparison for == with Approx(value).margin is actually two comparisons:

bool Approx::equalityComparisonImpl(const double other) const {
// First try with fixed margin, then compute margin based on epsilon, scale and Approx's value
// Thanks to Richard Harris for his help refining the scaled margin value
return marginComparison(m_value, other, m_margin)
|| marginComparison(m_value, other, m_epsilon * (m_scale + std::fabs(std::isinf(m_value)? 0 : m_value)));
}

And the second comparison margin (which is implicitly defined) for one instance is:

(lldb) print lhs
(double) 0.01044857591795904
(lldb) print rhs
(double) 0.010448575924245564
(lldb) print margin
(double) 1.2455673119972992E-7
(lldb) step
Process 93816 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = step in
    frame #0: 0x00000001000bf464 test_madx_elements`Catch::Detail::Approx::equalityComparisonImpl(this=0x000000016fdf9db8, other=0.010448575924245564) const at catch.hpp:7905:16
   7902	       // First try with fixed margin, then compute margin based on epsilon, scale and Approx's value
   7903	       // Thanks to Richard Harris for his help refining the scaled margin value
   7904	       return marginComparison(m_value, other, m_margin)
-> 7905	           || marginComparison(m_value, other, m_epsilon * (m_scale + std::fabs(std::isinf(m_value)? 0 : m_value)));
   7906	   }
   7907	
   7908	   void Approx::setMargin(double newMargin) {
(lldb) print this->m_margin
(double) 9.9999999999999999E-15
(lldb) 

where the implicitly defined margin is greater than the specified margin!

m_epsilon is defined as m_epsilon( std::numeric_limits<float>::epsilon()*100 ) per

: m_epsilon( std::numeric_limits<float>::epsilon()*100 ),

On an M1 mac the value of epsilon is:

sasyed@MAC-140753 ~/D/p/temp> bat test2.cc 
───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: test2.cc
───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ #include <iomanip>
   2   │ #include <iostream>
   3   │ 
   4   │ int main() {
   5   │   auto val1 = std::numeric_limits<float>::epsilon() * 100;
   6   │ 
   7   │   std::cout << std::setprecision(15) << val1 << "\n";
   8   │ }
───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
sasyed@MAC-140753 ~/D/p/temp> clang++ --sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk test2.cc && ./a.out
1.19209289550781e-05
sasyed@MAC-140753 ~/D/p/temp> 

@s-sajid-ali
Copy link
Member Author

Related to catchorg/Catch2#1507

@s-sajid-ali s-sajid-ali marked this pull request as ready for review April 29, 2024 21:24
@s-sajid-ali s-sajid-ali merged commit 6c56c36 into devel3 Apr 29, 2024
6 checks passed
@s-sajid-ali s-sajid-ali deleted the catch2_v3 branch April 29, 2024 21:24
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.

1 participant