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

SonicTriton feature updates, improvements, bug fixes #34508

Merged
merged 13 commits into from Jul 19, 2021

Conversation

kpedro88
Copy link
Contributor

@kpedro88 kpedro88 commented Jul 15, 2021

PR description:

  • Update namespaces to be consistent with new Triton version
  • Add I/O compression feature
  • Add SSL authentication feature
  • Update image classification network examples
  • Remove unnecessary LogInfo messages and add basic verbosity control to SonicCore
  • Add options to control server image used in fallback server
  • Update default server image to Triton v2.11
  • Prevent producer code from calling toServer()/fromServer() more than once per event (bug fix)
  • Update and improve documentation

PR validation:

Ran unit tests and new examples.

This PR requires an associated cmsdist PR (to be submitted simultaneously) to compile: cms-sw/cmsdist#7142

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34508/24002

ERROR: Build errors found during clang-tidy run.

HeterogeneousCore/SonicTriton/interface/TritonService.h:13:10: error: 'grpc_client.h' file not found [clang-diagnostic-error]
#include "grpc_client.h"
         ^
Suppressed 269 warnings (268 in non-user code, 1 with check filters).
--
HeterogeneousCore/SonicTriton/interface/TritonData.h:15:10: error: 'grpc_client.h' file not found [clang-diagnostic-error]
#include "grpc_client.h"
         ^
Suppressed 338 warnings (337 in non-user code, 1 with check filters).
--
HeterogeneousCore/SonicTriton/interface/TritonData.h:15:10: error: 'grpc_client.h' file not found [clang-diagnostic-error]
#include "grpc_client.h"
         ^
Suppressed 369 warnings (368 in non-user code, 1 with check filters).
--
HeterogeneousCore/SonicTriton/interface/TritonData.h:15:10: error: 'grpc_client.h' file not found [clang-diagnostic-error]
#include "grpc_client.h"
         ^
Suppressed 507 warnings (506 in non-user code, 1 with check filters).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@kpedro88
Copy link
Contributor Author

test parameters:
pull_request = cms-sw/cmsdist#7142

@kpedro88
Copy link
Contributor Author

the code-checks will not pass until the tests for cms-sw/cmsdist#7142 finish and provide an external build that can be used; then this PR can be tested

@kpedro88
Copy link
Contributor Author

code-checks with cms.week1.PR_5281fe91/46.0-5ad1edc02a2379b0e17209f83d607eda

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34508/24023

  • This PR adds an extra 8KB to repository

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34508/24032

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #34508 was updated. @makortel, @cmsbuild, @fwyzard can you please check and sign again.

@kpedro88
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8905a2/16924/summary.html
COMMIT: 3ba3cc0
CMSSW: CMSSW_12_0_X_2021-07-16-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34508/16924/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2786302
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2786273
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@kpedro88
Copy link
Contributor Author

@makortel if there's nothing further, I'd like to get this into pre5. thanks!

@makortel
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Jul 19, 2021

+1

@cmsbuild cmsbuild merged commit 4f7c86f into cms-sw:master Jul 19, 2021
@kpedro88
Copy link
Contributor Author

@qliphy @perrotta @smuzaffar cms-sw/cmsdist#7142 needs to be merged as well or the IB will break

@qliphy
Copy link
Contributor

qliphy commented Jul 19, 2021

@qliphy @perrotta @smuzaffar cms-sw/cmsdist#7142 needs to be merged as well or the IB will break

@kpedro88 Sorry I overlooked cms-sw/cmsdist#7142
@smuzaffar I saw your triggered test in cms-sw/cmsdist#7142 and it is still running. As it has already been tested with this PR, could we merge it already now?

@smuzaffar
Copy link
Contributor

cms-sw/cmsdist#7142 is merged now. I ran tests for all archs to make sure that new externals builds fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants