Skip to content

Compare output with result description for framework tests #91

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Compare output with result description for framework tests #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Input/input_sup2.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
<Parameter name="Delta" type="double" value="15.0"/>
<Parameter name="Tolerance" type="double" value="0.01"/>
</ParameterList>
</ParameterList>

<ParameterList name="result_description">
<Parameter name="ExpectedPressure" type="double" value="0.0060690998"/>
<Parameter name="ExpectedPressureTolerance" type="double" value="1e-6"/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tolerance is quite high compared to the value, can we set this to a smaller value and the tests still pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use the current output in the ExpectedPressure. I can set it by printing the current output using std::setprecision(16). And then I can use a lower tolerance. I am not sure what is a good value for this tolerance. Should I use 1e-10? The test still passes if I use this tolerance. I also intentionally broke the code by using an incorrect value for the expected result and it works.

</ParameterList>

</ParameterList>
5 changes: 5 additions & 0 deletions Input/input_sup5.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
<Parameter name="Delta" type="double" value="10.0"/>
<Parameter name="Tolerance" type="double" value="0.01"/>
</ParameterList>
</ParameterList>

<ParameterList name="result_description">
<Parameter name="ExpectedPressure" type="double" value="0.00086908959"/>
<Parameter name="ExpectedPressureTolerance" type="double" value="1e-6"/>
</ParameterList>

</ParameterList>
5 changes: 5 additions & 0 deletions Input/input_sup6.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
<Parameter name="Delta" type="double" value="10.0"/>
<Parameter name="Tolerance" type="double" value="0.01"/>
</ParameterList>
</ParameterList>

<ParameterList name="result_description">
<Parameter name="ExpectedPressure" type="double" value="0.00059421012"/>
<Parameter name="ExpectedPressureTolerance" type="double" value="1e-6"/>
</ParameterList>

</ParameterList>
5 changes: 5 additions & 0 deletions Input/input_sup7.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
<Parameter name="Delta" type="double" value="10.0"/>
<Parameter name="Tolerance" type="double" value="0.01"/>
</ParameterList>
</ParameterList>

<ParameterList name="result_description">
<Parameter name="ExpectedPressure" type="double" value="0.00055474375"/>
<Parameter name="ExpectedPressureTolerance" type="double" value="1e-6"/>
</ParameterList>

</ParameterList>
15 changes: 13 additions & 2 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@ int main(int argc, char* argv[])
int MaxIteration = 0;
bool PressureGreenFunFlag = false;

double ExpectedPressure = -1.0;
double ExpectedPressureTolerance = -1.0;

MIRCO::SetParameters(E1, E2, LateralLength, nu1, nu2, CompositeYoungs, CompositePoissonsRatio,
ShapeFactor, ElasticComplianceCorrection, GridSize, Tolerance, Delta, TopologyFilePath,
Resolution, InitialTopologyStdDeviation, inputFileName, RandomTopologyFlag, Hurst,
RandomSeedFlag, RandomGeneratorSeed, WarmStartingFlag, MaxIteration, PressureGreenFunFlag);
RandomSeedFlag, RandomGeneratorSeed, WarmStartingFlag, MaxIteration, PressureGreenFunFlag,
ExpectedPressure, ExpectedPressureTolerance);

// Identical Vectors/Matricies, therefore only created one here.
int ngrid = int(ceil((LateralLength - (GridSize / 2)) / GridSize));
Expand Down Expand Up @@ -67,6 +71,13 @@ int main(int argc, char* argv[])

const auto finish = std::chrono::high_resolution_clock::now();
const double elapsedTime =
std::chrono::duration_cast<std::chrono::seconds>(finish - start).count();
std::chrono::duration_cast<std::chrono::duration<double>>(finish - start).count();
Comment on lines -70 to +74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated to this merge request. Was this a bug you found on the side?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is not a bug. Earlier, the time taken was printed in integers of seconds. So, for very small runs, the elapsed time showed zero seconds. Therefore I changed this to see the time elapsed in floating value in seconds.

std::cout << "Elapsed time is: " + std::to_string(elapsedTime) + "s." << std::endl;

// Test for correct output if the result_description is given in the input file
if (ExpectedPressure >= 0)
{
TEUCHOS_TEST_FOR_EXCEPTION(std::abs(pressure - ExpectedPressure) > ExpectedPressureTolerance,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong test macro. You are testing if the code raises a std::runtime_error. You would need something like TEST_FLOATING_EQUALITY.

Side note: When implementing tests, always try to intentionally break the code and see if the test catches it. I assume in this case if you put the expected pressure to a wrong value the test will still pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I will change the test macro. However, the current test macro worked without any issues. I did intentionally break the code with an incorrect expected value and it does catch it.

std::runtime_error, "The output pressure is incorrect");
}
}
10 changes: 9 additions & 1 deletion src/mirco_setparameters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ void MIRCO::SetParameters(double& E1, double& E2, double& LateralLength, double&
double& ElasticComplianceCorrection, double& GridSize, double& Tolerance, double& Delta,
std::string& TopologyFilePath, int& Resolution, double& InitialTopologyStdDeviation,
const std::string& inputFileName, bool& RandomTopologyFlag, double& Hurst, bool& RandomSeedFlag,
int& RandomGeneratorSeed, bool& WarmStartingFlag, int& MaxIteration, bool& PressureGreenFunFlag)
int& RandomGeneratorSeed, bool& WarmStartingFlag, int& MaxIteration, bool& PressureGreenFunFlag,
double& ExpectedPressure, double& ExpectedPressureTolerance)
Comment on lines 15 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting quite long. This should be replaced with a parameter container in the future, to improve readability and usability.

{
Teuchos::RCP<Teuchos::ParameterList> parameterList = Teuchos::rcp(new Teuchos::ParameterList());
Teuchos::updateParametersFromXmlFile(inputFileName, parameterList.ptr());
Expand Down Expand Up @@ -94,4 +95,11 @@ void MIRCO::SetParameters(double& E1, double& E2, double& LateralLength, double&

ElasticComplianceCorrection = LateralLength * CompositeYoungs / ShapeFactor;
GridSize = LateralLength / (pow(2, Resolution) + 1);

if (parameterList->isSublist("result_description"))
{
Teuchos::ParameterList& result_description = parameterList->sublist("result_description");
ExpectedPressure = result_description.get<double>("ExpectedPressure");
ExpectedPressureTolerance = result_description.get<double>("ExpectedPressureTolerance");
}
}
4 changes: 3 additions & 1 deletion src/mirco_setparameters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ namespace MIRCO
* @param MaxIteration Maximum number of iterations for the force to converge.
* @param PressureGreenFunFlag Flag to use Green function based on uniform pressure instead of
* point force
* @param ExpectedPressure Expected pressure output for framework test
* @param ExpectedPressureTolerance Tolerance to compare the output to the expected pressure
*/
void SetParameters(double& E1, double& E2, double& LateralLength, double& nu1, double& nu2,
double& CompositeYoungs, double& CompositePoissonsRatio, double& alpha,
double& ElasticComplianceCorrection, double& GridSize, double& Tolerance, double& Delta,
std::string& TopologyFilePath, int& Resolution, double& InitialTopologyStdDeviation,
const std::string& inputFileName, bool& RandomTopologyFlag, double& Hurst,
bool& RandomSeedFlag, int& RandomGeneratorSeed, bool& WarmStartingFlag, int& MaxIteration,
bool& PressureGreenFunFlag);
bool& PressureGreenFunFlag, double& ExpectedPressure, double& ExpectedPressureTolerance);
} // namespace MIRCO

#endif // SRC_SETPARAMETERS_H_