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

Fix gen_doc build option and refresh documentation #3545

Merged
merged 23 commits into from
Apr 17, 2020
Merged

Conversation

hariharans29
Copy link
Member

Description: gen_doc build option was broken. This change fixes it. Enforcing doc updates with build breaks if doc updates are not checked-in as part of each relevant PR is still WIP but this PR has some changes to eventually enable that.

Motivation and Context
MQ work-item

@hariharans29 hariharans29 requested a review from a team as a code owner April 16, 2020 02:14
@@ -391,17 +391,6 @@ void addGlobalMethods(py::module& m, const Environment& env) {
"get_all_opkernel_def", []() -> const std::vector<onnxruntime::KernelDef> {
std::vector<onnxruntime::KernelDef> result;

// default logger is needed to create the DNNLExecutionProvider
std::string default_logger_id{"DefaultLogger"};
Copy link
Member Author

Choose a reason for hiding this comment

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

default logger is already created by the time execution flow reaches this line

@@ -868,15 +868,15 @@ def generate_documentation(source_dir, build_dir, configs):
operator_doc_path = os.path.join(source_dir, 'docs', 'ContribOperators.md')
opkernel_doc_path = os.path.join(source_dir, 'docs', 'OperatorKernels.md')
for config in configs:
#copy the gen_doc.py
shutil.copy(os.path.join(source_dir,'tools','python','gen_doc.py'),
#copy the gen_contrib_doc.py
Copy link
Member Author

Choose a reason for hiding this comment

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

renamed file to a name more indicative of its function

@@ -930,7 +930,7 @@ def main():
if args.use_tensorrt:
args.use_cuda = True

if args.build_wheel:
if args.build_wheel or args.gen_doc:
args.enable_pybind = True
Copy link
Member Author

Choose a reason for hiding this comment

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

pybind is to be enabled even if user doesn't state it as op schema/kernel def are available to script via Python bindings

@@ -15,7 +15,6 @@

import numpy as np # type: ignore

import onnxruntime as rt
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't need ORT in either of the scripts

@@ -41,7 +40,7 @@ def format_param_strings(params):
firstparam = True
s = ''
if params:
for param in params:
for param in sorted(params):
Copy link
Member Author

Choose a reason for hiding this comment

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

sort wherever possible to avoid order ambiguity and thus avoid False Positive diffs in the md files

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