forked from cms-sw/cmssw
-
Notifications
You must be signed in to change notification settings - Fork 2
HeterogeneousCore/SonicTriton: add RetryActionDiffServer; expose connectToServer; update tests #21
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
Draft
trevin-lee
wants to merge
10
commits into
fastmachinelearning:master
Choose a base branch
from
trevin-lee:SonicRetry_CMSSW_15_0_0_pre3
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
c1dce2b
RetryAction compiles
4dde1c6
Include RetryAction in SonicClientBase
5b093e4
Update PR comments
c062112
PR comments, fix fillDescriptions
ca570d7
Move RetrySameServerAction to plugins. SonicTriton test works.
fe03eb4
Add update server function for client
7586344
Add test for Triton retry action in BuildFile.xml
trevin-lee ab9b098
Implement retry logic in RetryActionDiffServer and add connectToServe…
trevin-lee 5b2c3c3
Add RetryActionDiffServer class documentation, implement testing cons…
trevin-lee 0212e70
SonicTriton: implement retry action against different server; update …
trevin-lee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
#ifndef HeterogeneousCore_SonicCore_RetryActionBase | ||
#define HeterogeneousCore_SonicCore_RetryActionBase | ||
|
||
#include "FWCore/PluginManager/interface/PluginFactory.h" | ||
#include "FWCore/ParameterSet/interface/ParameterSet.h" | ||
#include "HeterogeneousCore/SonicCore/interface/SonicClientBase.h" | ||
#include <memory> | ||
#include <string> | ||
|
||
// Base class for retry actions | ||
class RetryActionBase { | ||
public: | ||
RetryActionBase(const edm::ParameterSet& conf, SonicClientBase* client); | ||
virtual ~RetryActionBase() = default; | ||
|
||
bool shouldRetry() const { return shouldRetry_; } // Getter for shouldRetry_ | ||
|
||
virtual void retry() = 0; // Pure virtual function for execution logic | ||
virtual void start() = 0; // Pure virtual function for execution logic for initialization | ||
|
||
protected: | ||
void eval(); // interface for calling evaluate in client | ||
|
||
protected: | ||
SonicClientBase* client_; | ||
bool shouldRetry_; // Flag to track if further retries should happen | ||
}; | ||
|
||
// Define the factory for creating retry actions | ||
using RetryActionFactory = | ||
edmplugin::PluginFactory<RetryActionBase*(const edm::ParameterSet&, SonicClientBase* client)>; | ||
|
||
#endif | ||
|
||
#define DEFINE_RETRY_ACTION(type) DEFINE_EDM_PLUGIN(RetryActionFactory, type, #type); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<use name="FWCore/Framework"/> | ||
<use name="FWCore/PluginManager"/> | ||
<use name="FWCore/ParameterSet"/> | ||
<use name="HeterogeneousCore/SonicCore"/> | ||
<library file="*.cc" name="pluginHeterogeneousCoreSonicCore"/> | ||
|
30 changes: 30 additions & 0 deletions
30
HeterogeneousCore/SonicCore/plugins/RetrySameServerAction.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#include "HeterogeneousCore/SonicCore/interface/RetryActionBase.h" | ||
#include "HeterogeneousCore/SonicCore/interface/SonicClientBase.h" | ||
|
||
class RetrySameServerAction : public RetryActionBase { | ||
public: | ||
RetrySameServerAction(const edm::ParameterSet& pset, SonicClientBase* client) | ||
: RetryActionBase(pset, client), allowedTries_(pset.getUntrackedParameter<unsigned>("allowedTries", 0)) {} | ||
|
||
void start() override { tries_ = 0; }; | ||
|
||
protected: | ||
void retry() override; | ||
|
||
private: | ||
unsigned allowedTries_, tries_; | ||
}; | ||
|
||
void RetrySameServerAction::retry() { | ||
++tries_; | ||
//if max retries has not been exceeded, call evaluate again | ||
if (tries_ < allowedTries_) { | ||
eval(); | ||
return; | ||
} else { | ||
shouldRetry_ = false; // Flip flag when max retries are reached | ||
edm::LogInfo("RetrySameServerAction") << "Max retry attempts reached. No further retries."; | ||
} | ||
} | ||
|
||
DEFINE_RETRY_ACTION(RetrySameServerAction) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
#include "HeterogeneousCore/SonicCore/interface/RetryActionBase.h" | ||
|
||
// Constructor implementation | ||
RetryActionBase::RetryActionBase(const edm::ParameterSet& conf, SonicClientBase* client) | ||
: client_(client), shouldRetry_(true) {} | ||
|
||
void RetryActionBase::eval() { | ||
if (client_) { | ||
client_->evaluate(); | ||
} else { | ||
edm::LogError("RetryActionBase") << "Client pointer is null, cannot evaluate."; | ||
} | ||
} | ||
|
||
EDM_REGISTER_PLUGINFACTORY(RetryActionFactory, "RetryActionFactory"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,9 +7,13 @@ | |
<use name="HeterogeneousCore/CUDAUtilities"/> | ||
<use name="triton-inference-client"/> | ||
<use name="protobuf"/> | ||
<use name="catch2"/> | ||
<iftool name="cuda"> | ||
<use name="cuda"/> | ||
</iftool> | ||
|
||
<export> | ||
<lib name="1"/> | ||
<lib name="1"/> | ||
</export> | ||
|
||
<test name="RetryActionDiffServer_test" command="RetryActionDiffServer.cc"/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tests should be defined in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and the correct syntax is: <bin file="test_RetryActionDiffServer.cc" name="TestHeterogeneousCoreSonicTritonRetryActionDiffServer">
<use name="catch2"/>
<use name="FWCore/ParameterSet"/>
<use name="HeterogeneousCore/SonicTriton"/>
</bin> |
33 changes: 33 additions & 0 deletions
33
HeterogeneousCore/SonicTriton/interface/RetryActionDiffServer.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
#ifndef HeterogeneousCore_SonicTriton_RetryActionDiffServer_h | ||
#define HeterogeneousCore_SonicTriton_RetryActionDiffServer_h | ||
|
||
#include "HeterogeneousCore/SonicCore/interface/RetryActionBase.h" | ||
|
||
/** | ||
* @class RetryActionDiffServer | ||
* @brief A concrete implementation of RetryActionBase that attempts to retry an inference | ||
* request on a different, user-specified Triton server. | ||
* | ||
* This class is designed to provide a fallback mechanism. If an initial inference | ||
* request fails (e.g., due to server unavailability or a model-specific error), | ||
* this action will be triggered. It reads an alternative server URL from the | ||
* ParameterSet and instructs the TritonClient to reconnect to this new server | ||
* for the retry attempt. This action is designed for one-time use per inference | ||
* call; after the retry attempt, it disables itself until the next `start()` call. | ||
*/ | ||
|
||
class RetryActionDiffServer : public RetryActionBase { | ||
public: | ||
RetryActionDiffServer(const edm::ParameterSet& conf, SonicClientBase* client); | ||
~RetryActionDiffServer() override = default; | ||
|
||
void retry() override; | ||
void start() override; | ||
|
||
private: | ||
std::string alt_server_url_; | ||
std::string alt_server_token_; | ||
}; | ||
|
||
#endif | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be moved to
test/BuildFile.xml
as indicated in other comments (i.e. removed from here)