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

Update Performance attribute interfaces #271

Conversation

harimkang
Copy link
Contributor

@harimkang harimkang commented Aug 25, 2023

Modify to support performance of project REST API (Geti) keys
Related Issue: https://jira.devtools.intel.com/browse/CVS-118024

# Refer to new interface of client's web_ui/src/core/projects/dtos/project.interfaces.ts
export interface ScoreDTO {
    value: number;
    metric_type: string;
}

interface TaskPerformanceDTO {
    task_node_id: string;
    score: ScoreDTO | null;
    global_score?: ScoreDTO | null;
    local_score?: ScoreDTO | null;
}

export interface PerformanceDTO {
    score: number | null;
    global_score?: number | null;
    local_score?: number | null;
    task_performances: TaskPerformanceDTO[];
}

export interface ProjectDTO extends ProjectCommon {
    creation_time: string;
    id: string;
    name: string;
    datasets: DatasetDTO[];
    pipeline: {
        connections: ConnectionDTO[];
        tasks: TaskDTO[];
    };
    performance: PerformanceDTO;
    thumbnail: string;
    storage_info?: { size: number } | Record<string, never>;
}

@harimkang harimkang marked this pull request as ready for review August 25, 2023 01:40
@harimkang
Copy link
Contributor Author

harimkang commented Aug 25, 2023

Hi @ljcornel ,
I fixed this and checked locally that the inputs mentioned in the issue were passing fine, but now I see that the tests (including the main branch) also have issues.

There seem to be two issues,

  1. There is an update to the model_api 0.1.2, but it is not currently reflected. (Here's what I found while fixing bugs in the test.)

    • Need to change import openvino.model_zoo.model_api -> openvino.model_api
    • Need to change input of OpenvinoAdapter (param change: model_path -> model)
  2. However, even with the above fix, test throws an error because tests/data/dice/deployment.zip is out of date.

    • I asked kate, who added this, and she said that this needs to be manually updated with the deployment.zip, which we can get after running the training directly from the latest Geti.

So I realized that this had a bigger scope than I thought, so I stopped.

It seems to me that manually updating the deployment.zip in the tests folder is a very inefficient and time-consuming task. What do you think?

cc. @samet-akcay

@ljcornel
Copy link
Collaborator

ljcornel commented Aug 28, 2023

Hi @harimkang, thanks for the PR! Indeed, I have also noticed the problems with the tests, am working to fix them at the moment.

I agree with you that the test for the video reconstruction feature is not maintainable at the moment, updating the deployment data manually is not something we should be doing. I think I will skip this test for now until we find a better way to handle it.

Will let you know when the pipeline is working again, so that you can rebase your PR.

@ljcornel
Copy link
Collaborator

The CI is green again on main, @harimkang please rebase your PR to run the tests again.

…to harimkan/CVS-118024-performance-interface
@ljcornel ljcornel merged commit 9c83724 into openvinotoolkit:main Aug 29, 2023
3 checks passed
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