Skip to content

Conversation

andi4191
Copy link
Contributor

@andi4191 andi4191 commented May 10, 2022

Description

Added rtol + atol tolerance check for torchtrtc. Similar to what we have for test benchmarks.

This PR is a replica of #1038

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)
#1030

Type of change

Please delete options that are not relevant and/or add your own.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes

@andi4191 andi4191 requested a review from narendasan May 10, 2022 21:45
@andi4191 andi4191 self-assigned this May 10, 2022
@github-actions github-actions bot added component: api [C++] Issues re: C++ API documentation Improvements or additions to documentation labels May 10, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/cpp/bin/torchtrtc/main.cpp b/tmp/changes.txt
index e5a6c28..48d9eff 100644
--- a/workspace/cpp/bin/torchtrtc/main.cpp
+++ b/tmp/changes.txt
@@ -446,8 +446,8 @@ int main(int argc, char** argv) {
          threshold_ss << "atol: " << atol_val << " rtol: " << rtol_val;
          torchtrt::logging::log(
              torchtrt::logging::Level::kWARNING,
-              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") + threshold_ss.str() +
-                  std::string(")"));
+              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") +
+                  threshold_ss.str() + std::string(")"));
        }
      }
    } else {
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/cpp/bin/torchtrtc/main.cpp b/tmp/changes.txt
index e5a6c28..48d9eff 100644
--- a/workspace/cpp/bin/torchtrtc/main.cpp
+++ b/tmp/changes.txt
@@ -446,8 +446,8 @@ int main(int argc, char** argv) {
          threshold_ss << "atol: " << atol_val << " rtol: " << rtol_val;
          torchtrt::logging::log(
              torchtrt::logging::Level::kWARNING,
-              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") + threshold_ss.str() +
-                  std::string(")"));
+              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") +
+                  threshold_ss.str() + std::string(")"));
        }
      }
    } else {
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@andi4191 andi4191 changed the title Anuragd/torchtrtc atol rtol Using atol and rtol based tolerance threshold for torchtrtc May 10, 2022
@andi4191 andi4191 changed the title Using atol and rtol based tolerance threshold for torchtrtc feat: Using atol and rtol based tolerance threshold for torchtrtc May 10, 2022
@andi4191 andi4191 changed the title feat: Using atol and rtol based tolerance threshold for torchtrtc feat (//cpp): Using atol and rtol based tolerance threshold for torchtrtc May 10, 2022
@narendasan
Copy link
Collaborator

/blossom-ci

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/cpp/bin/torchtrtc/main.cpp b/tmp/changes.txt
index e5a6c28..48d9eff 100644
--- a/workspace/cpp/bin/torchtrtc/main.cpp
+++ b/tmp/changes.txt
@@ -446,8 +446,8 @@ int main(int argc, char** argv) {
          threshold_ss << "atol: " << atol_val << " rtol: " << rtol_val;
          torchtrt::logging::log(
              torchtrt::logging::Level::kWARNING,
-              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") + threshold_ss.str() +
-                  std::string(")"));
+              std::string("Maximum numerical deviation for output exceeds tolerance thresholds (") +
+                  threshold_ss.str() + std::string(")"));
        }
      }
    } else {
diff --git a/workspace/cpp/bin/torchtrtc/accuracy.cpp b/tmp/changes.txt
index 94592b0..4f99d17 100644
--- a/workspace/cpp/bin/torchtrtc/accuracy.cpp
+++ b/tmp/changes.txt
@@ -27,12 +27,9 @@ bool almost_equal(const at::Tensor& a, const at::Tensor& b, float atol, float rt
  auto result = diff.abs().max().item<float>();
  auto threshold = atol + (rtol * b.abs().max().item<float>());

+  torchtrt::logging::log(torchtrt::logging::Level::kDEBUG, std::string("Max Difference: ") + std::to_string(result));
  torchtrt::logging::log(
-      torchtrt::logging::Level::kDEBUG,
-      std::string("Max Difference: ") + std::to_string(result));
-  torchtrt::logging::log(
-      torchtrt::logging::Level::kDEBUG,
-      std::string("Acceptable Threshold: ") + std::to_string(threshold));
+      torchtrt::logging::Level::kDEBUG, std::string("Acceptable Threshold: ") + std::to_string(threshold));

  return result <= threshold;
}
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

@andi4191 andi4191 requested a review from narendasan May 25, 2022 21:11
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

…ce test

BREAKING CHANGE: The flag `--threshold` has been removed in favor of
two flags `--atol` and `--rtol` which control the maximum absolute
and relative tolerances for numberical deviation

Signed-off-by: Naren Dasan <naren@narendasan.com>
Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Anurag Dixit <a.dixit91@gmail.com>

fix: Fix for rtol and atol tolerance limit in torchtrtc

Signed-off-by: Anurag Dixit <a.dixit91@gmail.com>

feat(//cpp)!: Using logger instead of std::cout

Signed-off-by: Anurag Dixit <a.dixit91@gmail.com>

chore!: Applying C++ lint

Signed-off-by: Anurag Dixit <a.dixit91@gmail.com>

chore!: Updated the tensor names as per review comments

Signed-off-by: Anurag Dixit <a.dixit91@gmail.com>
@narendasan narendasan force-pushed the anuragd/torchtrtc_atol_rtol branch from 3cbe2e2 to 664f117 Compare June 22, 2022 19:54
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

@narendasan narendasan merged commit a94e493 into master Jun 22, 2022
@narendasan narendasan deleted the anuragd/torchtrtc_atol_rtol branch June 22, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: api [C++] Issues re: C++ API documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants