Skip to content

Conversation

@askhade
Copy link
Contributor

@askhade askhade commented Dec 10, 2018

No description provided.

@askhade askhade requested a review from a team December 10, 2018 19:50
@snnn
Copy link
Member

snnn commented Dec 10, 2018

/AzurePipelines run all

@azure-pipelines
Copy link

Success! 1 created and queued.

# fix a warning in onnx code we can't do anything about
if (MSVC)
target_compile_options(onnx_proto PRIVATE /wd4146) # unary minus operator applied to unsigned type
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -W1 -DEIGEN_HAS_C99_MATH /MP")
Copy link
Member

Choose a reason for hiding this comment

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

Why change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary for using Eigen erf. Without this compilation fails. You can check SpecialFunctionsImpl.h
Although now I do think this may not be the right place to add this... may be line 104 is a better place.

Copy link
Member

@snnn snnn Dec 10, 2018

Choose a reason for hiding this comment

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

-W1

Don't change the warning level globally. /W3 or /W4 is mandatory. You may disable a specific warning for a specific file or vc project.

-DEIGEN_HAS_C99_MATH

By default, Eigen strive to automatically detect and enable langage features at compile-time based on the information provided by the compiler. If it doesn't work, let's find out why. At lease we need a reason and a comment for this flag.

/MP

It's duplicated with line 301. Please remove it or merge them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ERF requires C++11 and C99
In case of MSVC C99 needs to be set explicitly

snnn
snnn previously approved these changes Dec 11, 2018
@askhade askhade merged commit b054646 into master Dec 11, 2018
@raymondxyang raymondxyang deleted the askhade/implement_erf branch December 14, 2018 19:01
souptc pushed a commit that referenced this pull request Dec 17, 2018
1. Remove "using namespace onnx" from header files
2.  onnx_test_runner: combine input/output data into a single file for each dataset.
3.  onnx_test_runner: change the way how finished event was sent, to fix a crash on exit bug.
4.  cmake: Cleanup include_directories, change it to target_include_directories, so that it only make effect on special targets.
5. cmake: remove lotusIR_graph_obj and lotus_common_obj target
6. cmake: perfer to use prebuilt zlib library
7. cloudtest: disable "LotusModelTest.resnet50.debug" and "LotusModelTest.vgg19.debug". They run too long.
8. cloudtest: add 6 models. coreml_SVC_sklearn_load_wine,coreml_SVC_sklearn_load_breast_cancer,coreml_SVC_OpenML_1464_blood_transfusion,coreml_SVC_OpenML_312_scene,coreml_SVR_sklearn_load_boston,coreml_SVR_sklearn_load_diabetes
9. logging: move the implementation of OStreamSink to a ".cc" file, so that users of it has no dependency on the "date" library.

Related work items: #137
souptc pushed a commit that referenced this pull request Dec 17, 2018
1. Remove using namespace from headers under lotus/core
2. Fix a tiny bug in ExecutionFrame::ReleaseMLValue function

Related work items: #137
souptc pushed a commit that referenced this pull request Dec 17, 2018
…on.h

Remove using namespace from mean_variance_normalization.h

Added by @chasun by mistake.

Related work items: #137
qti-yuduo pushed a commit to CodeLinaro/onnxruntime that referenced this pull request Aug 8, 2025
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.

4 participants