Merging new tests by Amiable-Vagabond · Pull Request #60 · NatLabRockies/SolTrace

Amiable-Vagabond · 2025-03-11T23:24:41Z

Merging new tests into base repo. These include:
st_hfsf_sim_run_test.cpp - high flux solar furnace sample
st_parabola_sim_run_test.cpp - parabola sample (doesn't work and is commented out)
st_powertower_sim_run_noopt_test.cpp - powertower sample with no optimization
st_powertower_sim_run_opt_test.cpp - powertower sample with optimization

… CMake to use cpp 17.

… tower test

…n on the powertower sample

…o see how it runs

google-tests/CMakeLists.txt

google-tests/soltrace_example_tests/st_powertower_sim_run_noopt_test.cpp

qualand · 2025-03-13T22:47:57Z

Why does actions only show 1 test? and why does it not use the test names?

Amiable-Vagabond · 2025-03-13T22:58:51Z

Why does actions only show 1 test? and why does it not use the test names?

Looks like this is because all tests are being compiled into a single executable. The ctest command has a verbose flag -VV, I'll add that to the github workflow file so it will print all outputs.

qualand

Thanks for putting this together! Please see my comments and we can discuss tomorrow.

google-tests/soltrace_example_tests/st_powertower_sim_run_opt_test.cpp

google-tests/CMakeLists.txt

qualand · 2025-03-14T20:52:35Z

Can you remove all the extra outputs in the tests?
for example
"
1: input file: /Users/runner/work/SolTrace/SolTrace/build/google-tests/High Flux Solar Furnace.stinput
1: loading input file version 2016.1.1
1: sun ps? 0 cs: p -0.499195 0.703243 -0.506215
1: stage 'Heliostat': [1] 0.04 1.284 27.339 -2.87652 5.10737 18.5711 0 0 0 0
1: stage 'Primary Mirror Array': [25] 0 0 0 0 0 1 0 0 0 0
1: stage 'Target': [1] 3.255 -0.851 6.175 0 0 0 0 1 0 0
1: Stage 1:
"

qualand

Please address the couple of comments and we should be good to go!

qualand · 2025-03-17T16:42:14Z

google-tests/csv_tools/split_csv.h

+#include <cstring>
+#include <vector>
+#include <exception>
+#include <cstdlib>


can you remove all unneeded libraries?

qualand · 2025-03-17T16:43:09Z

google-tests/soltrace_example_tests/st_hfsf_sim_run_test.cpp

+
+TEST(st_hfsf_sim_run_opt_test, BasicAssertions)
+{
+	string project_dir = PROJECT_DIR;


delete this line

qualand · 2025-03-17T16:44:18Z

google-tests/soltrace_example_tests/st_powertower_sim_run_noopt_test.cpp

+
+TEST(st_powertower_sim_run_noopt_test, BasicAssertions)
+{
+	string project_dir = PROJECT_DIR;


delete this line

qualand · 2025-03-17T16:45:03Z

google-tests/soltrace_example_tests/st_powertower_sim_run_opt_test.cpp

+
+TEST(st_powertower_sim_run_opt_test, BasicAssertions)
+{
+	string project_dir = PROJECT_DIR;


same as above

qualand

Sorry, I just saw gtest's guidance on this. Feel free use or change my suggestions.

qualand · 2025-03-17T19:13:12Z

google-tests/soltrace_example_tests/st_hfsf_sim_run_test.cpp

+
+using namespace std;
+
+TEST(st_hfsf_sim_run_opt_test, BasicAssertions)


Can you remove all underscores from all test suite names and test names?
See gtest FAQ: https://google.github.io/googletest/faq.html

Suggested change

TEST(st_hfsf_sim_run_opt_test, BasicAssertions)

TEST(HighFluxSolarFurnace, RaysTraced1e5OpticalErrorsOff)

qualand · 2025-03-17T19:14:34Z

google-tests/soltrace_example_tests/st_powertower_sim_run_noopt_test.cpp

+
+using namespace std;
+
+TEST(st_powertower_sim_run_noopt_test, BasicAssertions)


Suggested change

TEST(st_powertower_sim_run_noopt_test, BasicAssertions)

TEST(PowerTowerSurroundSingleFacet, RaysTraced5e5NoOpticalErrors)

qualand · 2025-03-17T19:16:08Z

google-tests/soltrace_example_tests/st_powertower_sim_run_opt_test.cpp

+
+using namespace std;
+
+TEST(st_powertower_sim_run_opt_test, BasicAssertions)


Suggested change

TEST(st_powertower_sim_run_opt_test, BasicAssertions)

TEST(PowerTowerSurroundSingleFacet, RaysTraced5e5NoOpticalErrorsPowTowerOptOn)

qualand · 2025-03-17T19:17:04Z

google-tests/soltrace_example_tests/st_sim_run_test.cpp

@@ -25,32 +14,23 @@ double roundToDecimalPlaces(double value, int decimalPlaces) {

 TEST(st_sim_run_test, BasicAssertions)


Suggested change

TEST(st_sim_run_test, BasicAssertions)

TEST(ApertureExamples, SingleRayNoOpticalErrors)

qualand · 2025-03-17T20:52:15Z

google-tests/soltrace_example_tests/high_flux_solar_furnace_test.cpp

+
+using namespace std;
+
+TEST(st_hfsf_sim_run_opt_test, BasicAssertions)


I think you missed this test for renaming.

Agh, I didn't add the change during last commit. Fixed shortly

…e name

Davis and others added 8 commits February 13, 2025 16:24

Created new test on power tower example file with 50000 rays. Changed…

16afa47

… CMake to use cpp 17.

Added ground results csv to new folder in google-tests directory

0b13176

Removed register prefix from variables in mtrand.h. More errors to come.

f966dc2

Merge remote-tracking branch 'upstream/develop' into develop

bffffea

Removed lines in CMakeLists causing linux failure, fixed bug in power…

a6b4b3c

… tower test

Created parabola.stinput test and a test using powertower optimizatio…

1ddbe33

…n on the powertower sample

Commented out parabola test for now, added nonexistent branch to CI t…

69d295b

…o see how it runs

Fixed syntax error

51b1c96

jmaack24 requested a review from qualand March 12, 2025 14:12

qualand reviewed Mar 13, 2025

View reviewed changes

google-tests/CMakeLists.txt Outdated Show resolved Hide resolved

qualand reviewed Mar 13, 2025

View reviewed changes

google-tests/soltrace_example_tests/st_powertower_sim_run_noopt_test.cpp Outdated Show resolved Hide resolved

qualand requested changes Mar 13, 2025

View reviewed changes

Changed tests based on PR feedback

f2990b1

Davis and others added 3 commits March 14, 2025 15:24

Fixed error in CMakeLists that caused failure on windows

0222f6b

Removed output messages from st_sim_run_test

a7e490b

Fixed silly mistake in previous commit

3ab51a6

Amiable-Vagabond requested a review from qualand March 17, 2025 15:51

qualand requested changes Mar 17, 2025

View reviewed changes

Added comments and removed unused libraries

93d105f

qualand requested changes Mar 17, 2025

View reviewed changes

Changed naming conventions of tests

fc9e291

qualand requested changes Mar 17, 2025

View reviewed changes

Amiable-Vagabond added 2 commits March 17, 2025 14:57

Added high flux solar furnace test changes, changed parabola test fil…

6e8f6c5

…e name

Fixed typo

d40654c

qualand approved these changes Mar 17, 2025

View reviewed changes

qualand merged commit b67574c into NatLabRockies:develop Mar 17, 2025
10 checks passed


		using namespace std;

		TEST(st_hfsf_sim_run_opt_test, BasicAssertions)

	TEST(st_hfsf_sim_run_opt_test, BasicAssertions)
	TEST(HighFluxSolarFurnace, RaysTraced1e5OpticalErrorsOff)


		using namespace std;

		TEST(st_powertower_sim_run_noopt_test, BasicAssertions)

	TEST(st_powertower_sim_run_noopt_test, BasicAssertions)
	TEST(PowerTowerSurroundSingleFacet, RaysTraced5e5NoOpticalErrors)


		using namespace std;

		TEST(st_powertower_sim_run_opt_test, BasicAssertions)

		@@ -25,32 +14,23 @@ double roundToDecimalPlaces(double value, int decimalPlaces) {

		TEST(st_sim_run_test, BasicAssertions)

	TEST(st_sim_run_test, BasicAssertions)
	TEST(ApertureExamples, SingleRayNoOpticalErrors)

Conversation

Amiable-Vagabond commented Mar 11, 2025

Uh oh!

Uh oh!

Uh oh!

qualand commented Mar 13, 2025

Uh oh!

Amiable-Vagabond commented Mar 13, 2025

Uh oh!

qualand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qualand commented Mar 14, 2025

Uh oh!

qualand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qualand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants