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

[VitisAI] fix graph save #21293

Merged
merged 1 commit into from
Jul 16, 2024
Merged

[VitisAI] fix graph save #21293

merged 1 commit into from
Jul 16, 2024

Conversation

BoarQing
Copy link
Contributor

@BoarQing BoarQing commented Jul 9, 2024

Description

Revert the wrong change in #20920

Motivation and Context

It would save the data at a wrong position

@BoarQing BoarQing changed the title fix graph save for vitisai [VitisAI] fix graph save Jul 9, 2024
@snnn
Copy link
Member

snnn commented Jul 9, 2024

bool result = model_proto->SerializeToOstream(output);

Here you cannot pass filename to a std function, as we assume all multibyte strings are utf-8 strings.
A std function expect a string in native encoding, i.e. ISO8859-1, CP936 etc.
So, here you also need to call ToPathString. Or, you may change this function's signature to use a different type for filename.


Refers to: onnxruntime/core/providers/vitisai/imp/graph.cc:127 in b1d0023. [](commit_id = b1d0023, deletion_comment = False)

@BoarQing
Copy link
Contributor Author

Hi, I fixed the conflict. Could you start the pipeline @snnn

@BoarQing
Copy link
Contributor Author

@HectorSVC Could you take a look. This is required for the release.

@HectorSVC
Copy link
Contributor

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,Windows x64 QNN CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed

@HectorSVC
Copy link
Contributor

/azp run Big Models,Linux Android Emulator QNN CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@HectorSVC HectorSVC merged commit dcc0436 into microsoft:main Jul 16, 2024
66 of 67 checks passed
@BoarQing BoarQing deleted the fix_graph_save branch July 25, 2024 15:07
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.

3 participants