Skip to content

Use relative errors by default when comparing output files#449

Open
PhilipFackler wants to merge 1 commit into
developfrom
PhilipFackler/rel-errors
Open

Use relative errors by default when comparing output files#449
PhilipFackler wants to merge 1 commit into
developfrom
PhilipFackler/rel-errors

Conversation

@PhilipFackler

Copy link
Copy Markdown
Collaborator

Description

Enable relative and absolute errors (for CSV comparison) and use relative by default.

Closes #368

Proposed changes

This enables aggregating absolute errors or relative errors. Optional parameters are added to the JSON solver input file. Scaling relative errors depends on the optional parameter "abs_err_threshold" below which the errors will switch to absolute (so we don't divide by zero).

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • [N/A] There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • [N/A] I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

@lukelowry lukelowry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just some comments. Thank you for making my life easier for validation. Much appreciated

* @brief Aggregate norm data for a single variable over time
*/
struct ErrorAggregate
struct TemporalNormAggregate

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I approve of this vocabulary

Comment thread GridKit/Testing/CSV.hpp
const std::string& test_file,
const std::string& reference_file,
ErrorType error_type = ErrorType::RELATIVE,
double abs_threshold = DEFAULT_ABS_ERROR_THRESHOLD);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of abs_threshold how about err_threshold since the type can change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is specific to computing relative error. If the reference value is lower than the threshold, you don't scale, so you switch to absolute.

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.

Need relative errors in aggregate errors

2 participants