Skip to content

Ensemble model#1535

Open
MohsenTaheriShalmani wants to merge 7 commits into
masterfrom
ai-ensemble
Open

Ensemble model#1535
MohsenTaheriShalmani wants to merge 7 commits into
masterfrom
ai-ensemble

Conversation

@MohsenTaheriShalmani
Copy link
Copy Markdown
Contributor

Implementation of the ensemble model

@Inject
lateinit var config: EMConfig

@Inject
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.

why are you using @Inject in this class? this class is not instantiated with an injector, isn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I think we need it because the injected fields are populated afterward viatest.injector.injectMembers(test)

* among all the other used models based on the average of key metrics.
* For the single-model scenario, the function simply returns the single model.
*/
private fun selectBestModel(endpoint: Endpoint): AIModel? {
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.

should return AIModel not AIModel?

// Return the single model if there is only one delegate.
if (delegates.size == 1) return delegates.first()

return delegates.maxByOrNull { model ->
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.

if there was no delegate, ie delegates.isEmpty, then should had crashed at init time


val bestModel = selectBestModel(input.endpoint)

val result = bestModel?.classify(input) ?: AIResponseClassification()
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.

bestModel should never be null

"Ensemble model is a combination of a comma-separated list, e.g., GLM,NN,KDE.")
var aiModelForResponseClassification: String = "NONE"

fun setAIModels(vararg models: AIResponseClassifierModel) {
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.

these functions shouldn't mixed up in the list of Cfg declaration. move for example to bottom of file with the other support functions

@Test
fun testRunDeterministic(){
testRunEM(AIResponseClassifierModel.DETERMINISTIC)
testRunEM("DETERMINISTIC")
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.

sorry, but don't like this. should avoid magic constant, and rather use enums. you can also have an overriden version of testRunEM where you take as input a list of enums, which then can convert internally to a join string with ,.


@Test
fun testRunEnsemble(){
testRunEM("GAUSSIAN,GLM,KDE,KNN,NN")
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.

testRunEM(listOf(AIResponseClassifierModel.GAUSSIAN,AIResponseClassifierModel.GLM,AIResponseClassifierModel.KDE,AIResponseClassifierModel.KNN,AIResponseClassifierModel.NN))

can also statically import AIResponseClassifierModel.* to avoid repeating it

@Cfg("Models used to learn input constraints and predict the response status before issuing a request. " +
"Supports both single-model and ensemble configurations. " +
"Ensemble model is a combination of a comma-separated list, e.g., GLM,NN,KDE.")
var aiModelForResponseClassification: String = "NONE"
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.

hmmm... converting to string lose all information on type checking, and automated documentation of available entries in the generated docs. need to support list of enums. but maybe easier if i implement it

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.

2 participants