Update expected DPS values in test #50390

BrettDong · 2021-08-01T14:28:19Z

Summary

None

Purpose of change

Tests are failing on DPS test.

Describe the solution

Update the expected values.

Describe alternatives you've considered

Testing

Wait and see if CI passes.

Additional context

BrettDong · 2021-08-01T14:52:02Z

The numbers yielded in "Basic Build and Test (GCC 9, Curses, LTO)" differs from what I get from my local build.

BrettDong · 2021-08-01T14:58:58Z

Result from my local build (master 37f9b6c):

-------------------------------------------------------------------------------
expected weapon dps
  clubs
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:396
...............................................................................

../tests/effective_dps_test.cpp:408: FAILED:
  CHECK( calc_expected_dps( "mace_inferior" ) == 17.5 )
with expansion:
  Approx( 18.3221610855 ) == 17.5

../tests/effective_dps_test.cpp:415: FAILED:
  CHECK( calc_expected_dps( "bokken_inferior" ) == 13.0 )
with expansion:
  Approx( 14.1142912141 ) == 13.0

-------------------------------------------------------------------------------
expected weapon dps
  shortswords
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:482
...............................................................................

../tests/effective_dps_test.cpp:496: FAILED:
  CHECK( calc_expected_dps( "cutlass_inferior" ) == 16.5 )
with expansion:
  Approx( 17.6306464034 ) == 16.5

../tests/effective_dps_test.cpp:499: FAILED:
  CHECK( calc_expected_dps( "wakizashi_inferior" ) == 14.0 )
with expansion:
  Approx( 15.0197079632 ) == 14.0

-------------------------------------------------------------------------------
expected weapon dps
  knives
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:514
...............................................................................

../tests/effective_dps_test.cpp:523: FAILED:
  CHECK( calc_expected_dps( "tanto_inferior" ) == 15.0 )
with expansion:
  Approx( 16.5113008386 ) == 15.0

../tests/effective_dps_test.cpp:533: FAILED:
  CHECK( calc_expected_dps( "tanto_fake" ) == 6.5 )
with expansion:
  Approx( 7.5104433501 ) == 6.5

===============================================================================
test cases:   1 |   0 passed | 1 failed
assertions: 302 | 296 passed | 6 failed

Result from "Basic Build / Basic Build and Test (GCC 9, Curses, LTO)" (this PR, 1 commit ahead master, 1e80860
):

(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> expected weapon dps
(~[slow] ~[.])=>   clubs
(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:396
(~[slow] ~[.])=> ...............................................................................
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:408: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "mace_inferior" ) == 18.5 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 17.4075931842 ) == 18.5
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:415: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "bokken_inferior" ) == 14.0 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 13.2080825309 ) == 14.0
(~[slow] ~[.])=> 
(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> expected weapon dps
(~[slow] ~[.])=>   shortswords
(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:482
(~[slow] ~[.])=> ...............................................................................
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:496: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "cutlass_inferior" ) == 17.5 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 16.7282112465 ) == 17.5
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:499: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "wakizashi_inferior" ) == 15.0 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 14.2525055255 ) == 15.0
(~[slow] ~[.])=> 
(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> expected weapon dps
(~[slow] ~[.])=>   knives
(~[slow] ~[.])=> -------------------------------------------------------------------------------
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:514
(~[slow] ~[.])=> ...............................................................................
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:523: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "tanto_inferior" ) == 16.5 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 15.0217614575 ) == 16.5
(~[slow] ~[.])=> 
(~[slow] ~[.])=> ../tests/effective_dps_test.cpp:533: FAILED:
(~[slow] ~[.])=>   CHECK( calc_expected_dps( "tanto_fake" ) == 7.5 )
(~[slow] ~[.])=> with expansion:
Error: (~[slow] ~[.])=>   Approx( 6.6075282444 ) == 7.5
(~[slow] ~[.])=> 
(~[slow] ~[.])=> 0.711 s: Vitamin Effects
(~[slow] ~[.])=> 1.017 s: recipe_permutations
(~[slow] ~[.])=> 0.775 s: archery_damage_thresholds
(~[slow] ~[.])=> ===============================================================================
(~[slow] ~[.])=> test cases:     693 |     692 passed | 1 failed
(~[slow] ~[.])=> assertions: 3683784 | 3683778 passed | 6 failed
(~[slow] ~[.])=>

anothersimulacrum · 2021-08-01T17:23:20Z

Results from master 37f9b6c:
make -j8 CCACHE=1 TILES=1 LINTJSON=0 RELEASE=1 RUNTESTS=1 DEBUG_SYMBOLS=1STRING_ID_DEBUG=1 ASTYLE=0

===============================================================================
All tests passed (3853326 assertions in 702 test cases)

Results from master 37f9b6c:
make -j8 CCACHE=1 TILES=0 LINTJSON=0 RELEASE=1 RUNTESTS=1 DEBUG_SYMBOLS=1 LTO=1 STRING_ID_DEBUG=1 ASTYLE=0

-------------------------------------------------------------------------------
starve_test
-------------------------------------------------------------------------------
../tests/stomach_contents_test.cpp:90
...............................................................................

../tests/stomach_contents_test.cpp:112: FAILED:
  REQUIRE( dummy.get_bmr() == 1738 )
with expansion:
  17380 (0x43e4) == 1,738 (0x6ca)
with messages:
  dummy.metabolic_rate_base() := 1.0f
  dummy.activity_level_str() := "EXTRA_EXERCISE"
  dummy.base_height() := 175
  dummy.get_size() := 3
  dummy.height() := 175
  dummy.get_bmi() := 25.0f
  dummy.bodyweight() := 76562500mg
  dummy.age() := 25
  dummy.get_bmr() := 17380 (0x43e4)

===============================================================================
test cases:     702 |     701 passed | 1 failed
assertions: 3855586 | 3855585 passed | 1 failed

Results from #50390 1e80860:
make -j8 CCACHE=1 TILES=0 LINTJSON=0 RELEASE=1 RUNTESTS=1 DEBUG_SYMBOLS=1 LTO=1 STRING_ID_DEBUG=1 ASTYLE=0

-------------------------------------------------------------------------------
starve_test
-------------------------------------------------------------------------------
../tests/stomach_contents_test.cpp:90
...............................................................................

../tests/stomach_contents_test.cpp:112: FAILED:
  REQUIRE( dummy.get_bmr() == 1738 )
with expansion:
  17380 (0x43e4) == 1,738 (0x6ca)
with messages:
  dummy.metabolic_rate_base() := 1.0f
  dummy.activity_level_str() := "EXTRA_EXERCISE"
  dummy.base_height() := 175
  dummy.get_size() := 3
  dummy.height() := 175
  dummy.get_bmi() := 25.0f
  dummy.bodyweight() := 76562500mg
  dummy.age() := 25
  dummy.get_bmr() := 17380 (0x43e4)

Log messages during failed test:
12:00:00AM: You eat your holy SPAM of debugging (fresh).
12:00:00AM: You start walking.

-------------------------------------------------------------------------------
expected weapon dps
  clubs
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:396
...............................................................................

../tests/effective_dps_test.cpp:408: FAILED:
  CHECK( calc_expected_dps( "mace_inferior" ) == 18.5 )
with expansion:
  Approx( 17.4075931842 ) == 18.5

../tests/effective_dps_test.cpp:415: FAILED:
  CHECK( calc_expected_dps( "bokken_inferior" ) == 14.0 )
with expansion:
  Approx( 13.2080825309 ) == 14.0

-------------------------------------------------------------------------------
expected weapon dps
  shortswords
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:482
...............................................................................

../tests/effective_dps_test.cpp:496: FAILED:
  CHECK( calc_expected_dps( "cutlass_inferior" ) == 17.5 )
with expansion:
  Approx( 16.7282112465 ) == 17.5

../tests/effective_dps_test.cpp:499: FAILED:
  CHECK( calc_expected_dps( "wakizashi_inferior" ) == 15.0 )
with expansion:
  Approx( 14.2525055255 ) == 15.0

-------------------------------------------------------------------------------
expected weapon dps
  knives
-------------------------------------------------------------------------------
../tests/effective_dps_test.cpp:514
...............................................................................

../tests/effective_dps_test.cpp:523: FAILED:
  CHECK( calc_expected_dps( "tanto_inferior" ) == 16.5 )
with expansion:
  Approx( 15.0217614575 ) == 16.5

../tests/effective_dps_test.cpp:533: FAILED:
  CHECK( calc_expected_dps( "tanto_fake" ) == 7.5 )
with expansion:
  Approx( 6.6075282444 ) == 7.5

===============================================================================
test cases:     702 |     700 passed | 2 failed
assertions: 3850576 | 3850569 passed | 7 failed

actual-nh · 2021-08-01T17:36:08Z

I suggest these probably need some rewriting - a combination of looser Approx requirements with use of ordering requirements between the real, inferior, and fake ones.

anothersimulacrum · 2021-08-01T17:38:25Z

I don't think that is the problem. We shouldn't be getting these different results with the same data.

actual-nh · 2021-08-01T17:44:01Z

I don't think that is the problem. We shouldn't be getting these different results with the same data.

I suggest seeing if it's unstable depending on the seed (if so, using a slightly better RNG may work).

I am also concerned about the get_bmr() wackiness.

anothersimulacrum · 2021-08-01T17:46:49Z

I can try RNG seed, but we're getting the same result to several digits:
From mine:
Approx( 17.4075931842 ) == 18.5
From the tests:
Approx( 17.4075931842 ) == 18.5
Approx( 17.4075931842 ) == 18.5

actual-nh · 2021-08-01T17:49:18Z

I can try RNG seed, but we're getting the same result to several digits:
From mine:
Approx( 17.4075931842 ) == 18.5
From the tests:
Approx( 17.4075931842 ) == 18.5
Approx( 17.4075931842 ) == 18.5

OTOH, BrettDong for mace_inferior had Approx( 18.3221610855 ) == 17.5.

anothersimulacrum · 2021-08-01T17:50:39Z

I think the difference is compiler/system as opposed to RNG.

actual-nh · 2021-08-01T17:51:22Z

I think the difference is compiler/system as opposed to RNG.

I see what you mean. Doing a compile locally.

actual-nh · 2021-08-01T20:25:19Z

I think the difference is compiler/system as opposed to RNG.

I see what you mean. Doing a compile locally.

Well, after fixing one bug (#50400), finally should be able to check the starve and/or DPS tests here.

anothersimulacrum · 2021-08-01T20:59:09Z

It is a clang/gcc thing - with clang, the tests pass with this PR applied. With gcc, they fail.
With clang, the tests fail on master - with gcc, they pass.

actual-nh · 2021-08-01T21:18:14Z

It is a clang/gcc thing - with clang, the tests pass with this PR applied. With gcc, they fail.
With clang, the tests fail on master - with gcc, they pass.

I can confirm that also happens with an older clang++ version. No starve test failure locally.

Compiling with -ffast-math causes clang and gcc to give different results for proportional values. e.g. gcc for 3.5 * 2, when stored as an integer, gives 6, whereas clang gives 7. Avoid this confusing behaviour and odd results in the DPS tests by dropping this option.

Remove the strong stomach immunity check in the tests, as it was removed from the game in a96f069

anothersimulacrum

Pending tests passing.

kevingranade

Code LGTM, just want a successful test run to confirm.

Update expected DPS values in test

1e80860

BrettDong added Code: Tests Measurement, self-control, statistics, balancing. Melee Melee weapons, tactics, techniques, reach attack labels Aug 1, 2021

BrettDong requested a review from anothersimulacrum August 1, 2021 14:36

actual-nh mentioned this pull request Aug 1, 2021

Friendly baby monsters disappear when they grow up #47145

Closed

BrettDong mentioned this pull request Aug 2, 2021

Fix unit tests #50405

Closed

anothersimulacrum added 2 commits August 2, 2021 10:13

Remove strong stomach section of effects test

97f9f05

Remove the strong stomach immunity check in the tests, as it was removed from the game in a96f069

anothersimulacrum approved these changes Aug 2, 2021

View reviewed changes

actual-nh added (P2 - High) High priority (for ex. important bugfixes) <Bugfix> This is a fix for a bug (or closes open issue) labels Aug 2, 2021

kevingranade approved these changes Aug 2, 2021

View reviewed changes

kevingranade marked this pull request as ready for review August 2, 2021 06:26

kevingranade merged commit 8b336f1 into CleverRaven:master Aug 2, 2021

BrettDong deleted the dps branch August 2, 2021 14:38

BrettDong mentioned this pull request Aug 2, 2021

Fix potentially incorrect floating-point arithmetic on MinGW #50426

Merged

BrettDong mentioned this pull request Jan 10, 2022

Revert "Workaround for linking error w/ clang & glibc 2.31+" #54241

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update expected DPS values in test #50390

Update expected DPS values in test #50390

BrettDong commented Aug 1, 2021

BrettDong commented Aug 1, 2021

BrettDong commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021 •

edited

anothersimulacrum left a comment

kevingranade left a comment

Update expected DPS values in test #50390

Update expected DPS values in test #50390

Conversation

BrettDong commented Aug 1, 2021

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

BrettDong commented Aug 1, 2021

BrettDong commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021

actual-nh commented Aug 1, 2021

anothersimulacrum commented Aug 1, 2021

actual-nh commented Aug 1, 2021 • edited

anothersimulacrum left a comment

Choose a reason for hiding this comment

kevingranade left a comment

Choose a reason for hiding this comment

actual-nh commented Aug 1, 2021 •

edited