Skip to content
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.

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

DLFW changes 25.01 onwards #3356

Merged
merged 4 commits into from
Mar 15, 2025
Merged

DLFW changes 25.01 onwards #3356

merged 4 commits into from
Mar 15, 2025

Conversation

apbose
Copy link
Collaborator

@apbose apbose commented Jan 15, 2025

Some minor changes for the NGC container release

@apbose apbose marked this pull request as draft January 15, 2025 01:28
@apbose apbose self-assigned this Jan 15, 2025
@github-actions github-actions bot added the component: tests Issues re: Tests label Jan 15, 2025
@github-actions github-actions bot requested a review from narendasan January 15, 2025 01:28
@apbose apbose force-pushed the DLFW_changes_25.01_onwards branch from 6fe0782 to 496e39d Compare February 24, 2025 18:53
@github-actions github-actions bot added the component: core Issues re: The core compiler label Feb 24, 2025
@apbose apbose marked this pull request as ready for review February 24, 2025 19:03
Copy link
Collaborator

@peri044 peri044 left a comment

Choose a reason for hiding this comment

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

LGTM. posted a minor suggestion

Comment on lines 1 to 9
#if defined(__GNUC__) && !defined(__clang__)
#if __GNUC__ >= 13
#include <cstdint>
#endif
#elif defined(__clang__)
#if __clang_major__ >= 13
#include <cstdint>
#endif
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider changing to

// Include cstdint for GCC 13+ or Clang 13+
#if (defined(__GNUC__) && !defined(__clang__) && (__GNUC__ >= 13)) || \
    (defined(__clang__) && (__clang_major__ >= 13))
    #include <cstdint>
#endif

noxfile.py Outdated
@@ -34,7 +34,7 @@
# Set epochs to train VGG model for accuracy tests
EPOCHS = 25

SUPPORTED_PYTHON_VERSIONS = ["3.9", "3.10", "3.11", "3.12"]
SUPPORTED_PYTHON_VERSIONS = ["3.7", "3.8", "3.9", "3.10", "3.12"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC we dont support py 3.7, 3.8 anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I will remove them then

@@ -4,9 +4,9 @@
import torch
import torch.nn as nn
import torch_tensorrt as torchtrt
import torch_tensorrt.ts.ptq as PTQ
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine to have but we should drop these tests from DLFW as this API is being deprecated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes they are dropped in DLFW

@apbose apbose force-pushed the DLFW_changes_25.01_onwards branch from afbeaba to 0039841 Compare March 14, 2025 04:13
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/home/runner/work/TensorRT/TensorRT/core/util/Exception.h b/tmp/changes.txt
index d5a856f..8f05056 100644
--- a/home/runner/work/TensorRT/TensorRT/core/util/Exception.h
+++ b/tmp/changes.txt
@@ -1,7 +1,6 @@
// Include cstdint for GCC 13+ or Clang 13+
-#if (defined(__GNUC__) && !defined(__clang__) && (__GNUC__ >= 13)) || \
-    (defined(__clang__) && (__clang_major__ >= 13))
-    #include <cstdint>
+#if (defined(__GNUC__) && !defined(__clang__) && (__GNUC__ >= 13)) || (defined(__clang__) && (__clang_major__ >= 13))
+#include <cstdint>
#endif
#pragma once

ERROR: Some files do not conform to style guidelines

@apbose apbose force-pushed the DLFW_changes_25.01_onwards branch from ea5229f to 7aea483 Compare March 14, 2025 05:28
@apbose apbose merged commit 7705d73 into main Mar 15, 2025
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: core Issues re: The core compiler component: tests Issues re: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants