[Good First Issue]: Check the validity of device name

[peterchen-intel]

Context

ov::Core{}.compile_model(path, "CPU -nstreams 1"); reports the error as expected: Device with "CPU -nstreams 1" name is not registered in the OpenVINO Runtime

ov::Core{}.compile_model(path, "MULTI:CPU(1),GPU(2) -nstreams 1"); must not be allowed should reports the error, but it doesn't.

What needs to be done?

• [ ] Create device name validate function in ov::util. Throw exception when any characters in device name are after parenthesis - like (4) except the device name separator - comma (,). For example, "MUTLTI:CPU(4),GPU(4)" is OK, but "MUTLTI:CPU(4),GPU(4) " or "MUTLTI:CPU(4),GPU(4)a" is incorrect.
• [ ] Call the validate function in ov::CoreImpl::compile_model
• [ ] Create test cases to cover correct and incorrect devices tests (src/tests/functional/plugin/shared/src/behavior/compiled_model/device.cpp) Reference https://github.com/openvinotoolkit/openvino/blob/84400513d514a095aeb4e11ec11ef79ed84ff47b/src/tests/functional/plugin/shared/src/behavior/compiled_model/properties.cpp

Example Pull Requests

No response

Resources

• Contribution guide - start here!
• Intel DevHub Discord channel - engage in discussions, ask questions and talk to OpenVINO developers

Contact points

@riverlijunjie, @ilya-lavrenov, @wangleis

Ticket

CVS-124122

No response

[awayzjj]

.take

[github-actions[bot]]

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

[kartik912]

This is my first contribution, can I take this issue. Any guidance will be welcome

[awayzjj]

This is my first contribution, can I take this issue. Any guidance will be welcome

Hello, I'm already working on this issue.

[awayzjj]

Hi @riverlijunjie, I have created a PR addressing the issue and look forward to your feedback on it. Another question, I can not find a proper file in ov::util to put the validate function, but found a similar function stripDeviceName in src/inference/src/dev/core_impl.cpp, so I put the validate function below it as below. I wonder if it is ok.

void stripDeviceName(std::string& device, const std::string& substr) {
    auto pos = device.find(substr);
    if (pos == 0) {
        device.erase(pos, substr.length());
    }
}

void validDeviceName(const std::string& device) {
    auto pos = device.rfind(")");
    if (pos != std::string::npos && pos != device.length() - 1) {
        OPENVINO_THROW("Device with \"", device, "\" name is illegal in the OpenVINO Runtime");
    }
}

[riverlijunjie]

@awayzjj thank your PR, we have some comments please help resolve them. Supposed the validate function is only called by ov::CoreImpl::compile_model, put it into core_impl.cpp is ok.

[ilya-lavrenov]

Supposed the validate function is only called by ov::CoreImpl::compile_model, put it into core_impl.cpp is ok.

Maybe we need to add it to https://github.com/openvinotoolkit/openvino/blob/c40b675f7638be40a1b863a1bf981ac4a7e0b65c/src/inference/src/dev/core_impl.cpp#L241-L243 ? It's called from all places where work with device name is performed.

[awayzjj]

I put the check in the parseDeviceNameIntoConfig and rewrote the test, since iefode reminded me that the test instance is not suitable to be placed in shared lib. Could you please help me review the PR? Thanks! @riverlijunjie @ilya-lavrenov @wangleis