enable mkldnn_fc layer and unit test #3096
Conversation
and use DNN_LOG_LEVEL
seems it is not required for FC
and enable the gtest
There was a problem hiding this comment.
Please also consider Mac OS. currently, MKLDNN doesn't support Mac OS X, it needs to be disabled under Mac OS X. uxlfoundation/oneDNN#91
| @@ -36,8 +36,8 @@ include(simd) | |||
| ################################ Configurations ####################################### | |||
| option(WITH_GPU "Compile PaddlePaddle with NVIDIA GPU" ${CUDA_FOUND}) | |||
| option(WITH_AVX "Compile PaddlePaddle with AVX intrinsics" ${AVX_FOUND}) | |||
There was a problem hiding this comment.
Please also consider Mac OS. currently, MKLDNN doesn't support Mac OS X, it needs to be disabled under Mac OS X. uxlfoundation/oneDNN#91
|
|
There was a problem hiding this comment.
个人理解，不对的地方请指正：
- paddle调用mkldnn库，layer.h/activation.h等头文件都不应该有变化，之前调用mk库，cuda/cudnn库，warp-ctc库都没有改基础的这些头文件。
- 从http://01org.github.io/mkl-dnn/ 看，结合以前Paddle调用高性能库的经验，新增的mkldnnfclayer的代码应该是不多的。现在这个layer的代码有600+，远超过Paddle其他的layer，而且风格上差异太多。不太理解，为什么调用一个库，会使layer的风格出现那么大的变化。
cmake/external/mkldnn.cmake
Outdated
|
|
||
| IF(WIN32) | ||
| MESSAGE(WARNING "It is not supported compiling with mkldnn in windows Paddle yet." | ||
| MESSAGE(WARNING "Windowns is not supported with MKLDNN in Paddle yet." |
| MESSAGE(WARNING "MacOS is not supported with MKLDNN in Paddle yet." | ||
| "Force WITH_MKLDNN=OFF") | ||
| SET(WITH_MKLDNN OFF CACHE STRING "Disable MKLDNN in MacOS" FORCE) | ||
| #SET(CMAKE_MACOSX_RPATH 1) # hold for MacOS |
There was a problem hiding this comment.
是可以删了，是为了以后支持mac留下的，现在用不到。
| #include "paddle/parameter/Argument.h" | ||
|
|
||
| #include "paddle/gserver/layers/MkldnnBase.h" | ||
| // #include "paddle/gserver/layers/MkldnnMemory.h" |
There was a problem hiding this comment.
可以删了，本来打算也是为后期实现activation留的
| : botData_(nullptr), | ||
| botDiff_(nullptr), | ||
| topData_(nullptr), | ||
| topDiff_(nullptr) {} |
There was a problem hiding this comment.
botData, botDiff这些定义类似caffe，不能用Paddle中已经定义好的变量么
There was a problem hiding this comment.
这个不是很方便用paddle已有的，已有是inputLayers_， 和output等，是一种buffer，这里的实际上用的是一个新class MkldnnBuffer，专门为paddle设计的， 他主要包含user和internal两部分，用于兼容输入输出不为mkldnn layer的转换，这点与Intelcaffe是类似的。
| if (!inputLayer->hasNextLayer(thisLayer)) { | ||
| inputLayer->addNextLayer(thisLayer); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
74-81行没必要加吧。cudnn layer和warpctc layer都没加。
There was a problem hiding this comment.
这个是特意要加的，原因是我发现现有paddle里面当前layer并不知道下一个layer是什么（如果我说的不对请帮忙纠正），引入他的目的是让当前更好的知道前后layer的是不是mkldnn的，如果不是，需要转换格式（比如nchw转mkldnn的内部格式）。所以你会看到我需要在调用mkldnn之前，先判断前后layer的mkldnn情况，并且还要考虑到输入或者输出有多个layer的，用于像resnet和googlenet的情况。
There was a problem hiding this comment.
- 当前layer不需要知道下一个layer是什么。
- 每个layer的输入和输出是一样的，转换格式应该在layer内部做，用来调用mkldnn库。
- 调用cudnn之前，也没有判断前后layer的情况。
There was a problem hiding this comment.
每个layer的输入和输出是一样的
每个layer的输入和输出不一定一样吧，我指的是layer type。
There was a problem hiding this comment.
我也认为当前Layer不需要知道下一个Layer是什么，每个Layer直观按照自己的格式输出就行，转换可以在下一个Layer里面做。另外，比如nchw转mkldnn的内部格式 mkldnn内部是什么样的格式？
There was a problem hiding this comment.
首先，我不是很明白，mklnnconv输出的nChw8c格式，后面的8c指的是什么意思？@tensor-tang这个能解释一下吗？我觉得这个是根本问题，比如理解mklnnconv+mklnnpoolin和mklnnconv+paddlepooling，以及mklnnconv+mklnnfc和mklnnconv+paddlefc这样的layer组合之间需要做什么数据转换都需要基于上面那个数据格式的理解。
There was a problem hiding this comment.
如果mkldnn的格式转换会带来很大的性能问题时，能否在data layer的时候就转成mkldnn格式。目前阶段也不考虑和其他非mkldnn layer的混用。
如果不考虑混用，那是会更容易。但是像DS2这种，就没法说实现一个优化的layer就用一个。对于某种网络就必须一套全用DNN了，这样基本就是两套API的意思了。如果现在不考虑的话，后期也再考虑只会更加麻烦了，并且也可能还是需要nextlayer信息。所以也就意味基本不会考虑了。
@hedaoyuan
nChw8c是mkldnn里面的一种格式，具体你可以参考这里
There was a problem hiding this comment.
@tensor-tang 你贴的那个地方只是一些枚举的定义。或者，有没有哪里描述nChw8c与nchw的转换关系？
There was a problem hiding this comment.
你说的转换关系是什么？指怎么转的吗？mkldnn有一个reorder函数是负责转这个的。具体格式针对你的那里例子其实不错的，是会有区别的，我在现在code里面是会把格式流打出来的叫printdataflow，但是因为只有fc，不会有很多信息打出来，明天我发一个稍微全一点的给你，可以有直观点的认识。
There was a problem hiding this comment.
实际上，我看了一下代码，还是没有理解nChw8c中8c的具体含义，是channel必须是8的倍数的意思？看了一下，分配内存的地方是在这里，但compute_blocking这个接口好像没有暴露，所以还是不明白nChw8c是分配了一个什么样的数据结构。
| @@ -105,6 +105,147 @@ class Layer { | |||
| std::vector<std::shared_ptr<FunctionBase>> backward_; | |||
|
|
|||
| public: | |||
| #ifdef PADDLE_USE_MKLDNN | |||
There was a problem hiding this comment.
为什么使用了mkldnn库后，需要加入那么多新的变量和函数呢？我理解的是，layer.h应该是不变的，之前加cudnn layer和集成第三方库warp-ctc的时候，layer.h
也是没有变的。
There was a problem hiding this comment.
首先，加入这个函数接口默认是不会改变paddle原本的逻辑，virtual的都是默认空的。其次，原本我也是不想加在父类的，但是尝试了一段时间之后，发现有些调用必须是基于父类的，要看做一个未知的layer来加以判断是不是mkldnn，其主要目的一方面也是要更好的支持paddle任何layer与mkldnn的混用，另外一方面也是在混用的同时提高性能。不然，其实直接不管输入输出是不是mkldnn，我都可以以默认格式来，那就会有很多没有必要的转换。所以才从父类加入了这些判断。核心的函数其实是在mkldnn会override的，父类只是起到一个接口的作用，不会引入太多逻辑。
There was a problem hiding this comment.
不会引入太多的逻辑
这段新增的代码有100多行，逻辑已经非常多了。新增库不应该在layer.h中增加逻辑。库的逻辑不应该在这里体现。会给后续维护增加困难。
直接不管输入输出是不是mkldnn，我都可以以默认格式来，那就会有很多没有必要的转换
输入输出应该是paddle默认的格式。调用mkl库的时候，也不管输入输出是不是mkl，为什么mkldnn需要这样呢？
在混用的同时提高性能
可以理解能提高性能，但这部分性能会有多少的提高？现在这样的写法，使得有两套数据结构，后续的维护会非常麻烦。
There was a problem hiding this comment.
输入输出应该是paddle默认的格式。调用mkl库的时候，也不管输入输出是不是mkl，为什么mkldnn需要这样呢？
mkldnn里面有一种内部格式，默认在这个格式下执行效率是最高，高于paddle上的nchw。所以才存在格式的转换。
可以理解能提高性能，但这部分性能会有多少的提高？现在这样的写法，使得有两套数据结构，后续的维护会非常麻烦。
不会有两套数据结构啊，这个不是很明白。 这个性能是可想而知，按照你的逻辑每个iteration做完都需要转换的，从性能上是非常得不偿失的。
There was a problem hiding this comment.
引入'MKLNN'应该是针对一些特殊Layer的修改。
另外，这个PR是enable mkldnn_fc layer，但是这里很多逻辑看起来跟fc layer并没有关系。
There was a problem hiding this comment.
- 两套格式：一套是paddle自己的格式，一套是mkldnn的格式。
- 如果不做转换，其他没有用mkldnn的layer怎么接用了mkldnn的layer。
- 目前写法是判断上下layer是不是mkldnn。在paddle中，layer是不知道下面的layer信息的。paddle后续重构后，op也是不知道后续op的任何信息的。
- 如果要使用这个格式的话，从data layer进来就要转成mkldnn的内部格式，依次往下传递，这样就不需要每次都转换
There was a problem hiding this comment.
这个PR如果可以的话，建议拆成2个：
- 第一个PR是加mklbase的一些信息，包括数据结构的定义等？即如何让paddle简洁明了地调用mkldnn库。建议可以在这个PR之前再先写一个design doc。
- fc的代码可以在下一个PR中交。
There was a problem hiding this comment.
这里面好像都不止两个PR，至少Layer这个层面有fc相关的，有sequence相关的，还有上面提到的nchw这个是图像数据和卷积计算相关的，这些特性最好分不同的pr来添加吧。
There was a problem hiding this comment.
@luotao1 @hedaoyuan
关于拆分这一点我们之前也讨论了，实现一个fc layer需要包括基本和实际fc，分开之后没法单独验证，所以我是真觉得不好拆分了。能拆分的就是后面准备提交的每个layer了。并且我之前就是考虑到这次code量有点多，所以我才把FC放在第一个，因为他比较简单，能比较容易的分清楚哪些是base的那些是fc的。
建议可以在这个PR之前再先写一个design doc
design doc实际在之前的邮件中我已经发过了，已经说明了我的想法以及准备的操作方式，并且是得到Baidu同事的认同，之后我才开始实施的。
另外 @hedaoyuan 这里的nchw格式也不仅仅是卷积相关的，在mkldnn里面就是众多格式中的一种，可以看做是通用的，他也可以是别的格式，不完全是layer相关的。所以我认为不能看做一种特性来提交。
There was a problem hiding this comment.
1.关于拆分这一点：之前是偏向拆分，但后来也是同意不拆分的。可是没想到这个PR有3000多行的改动。3000多行的代码肯定是可以拆分的。分开之后肯定也是能单独验证的。
2. 邮件中的不是design doc，是计划书。design doc是接口设计，数据格式设计等。
|
|
||
| namespace paddle { | ||
|
|
||
| REGISTER_LAYER(mkldnn_fc, MkldnnFcLayer); |
There was a problem hiding this comment.
可能我对mkldnn不是很了解，但从http://01org.github.io/mkl-dnn/ 上看，增加一个mkldnn的卷积层代码量是很少的。为什么fc层这里需要那么长的代码？
There was a problem hiding this comment.
这里我需要解释下，mkldnn本身的使用其实不会很复杂，但是也需要定义primivite desc和memory desc这是必须的。但是融入到framework当中就需要很多兼容问题，这一点我们其实是与Intelcaffe类似的。你会发现Intelcaffe 关于mkldnn的layer的setup函数也是需要一些准备工作的，除此之外，在framework内部也融入了mkldnn memory的接口，定义了一些接口方便用于mkldnn相关layer的使用。这里我把这些setup流程化了，分成了一些函数，一部分是方便在paddle中debug，另外一部分也是方面以后写新layer，只需要写实现就可以了，也方便后期开发者更改。
ps: 这里关于IntelCaffe的说法仅仅是个人看法，不代表官方说法，请勿转发，谢谢。
There was a problem hiding this comment.
- 兼容问题能不能详细描述下？
- intel caffe中的fc https://github.com/intel/caffe/blob/master/src/caffe/layers/mkldnn_inner_product_layer.cpp 实现也非常长，不可以简化么？
There was a problem hiding this comment.
Intelcaffe的相关实现，不方便在这里过多评论。
兼容问题其实就是指的格式转换问题，目的也就是让paddle既能保证mkldnn的性能又能保持与不是mkldnn layer的混用。
There was a problem hiding this comment.
paddle既能保证mkldnn的性能又能保持与不是mkldnn layer的混用。
merge进framework的时候，也要保持framework原有的设计逻辑吧。
| << ",cudnn_version=" << hl_get_cudnn_lib_version(); | ||
| #ifndef PADDLE_USE_MKLDNN | ||
| CHECK(!FLAGS_use_mkldnn) << "Can not use mkldnn, please set WITH_MKLDNN=ON"; | ||
| #endif |
There was a problem hiding this comment.
这里能处理成和cudnn_version一样么？不要加ifdef
There was a problem hiding this comment.
不是很明白你的意思，我的目的是判断如果编译时没开启mkldnn，就不能使用mkldnn，这里更version没什么关系。或者可以帮我指明下，GPU是在哪里做这个check的吗，我去那里加也可以。
There was a problem hiding this comment.
没开启mkldnn的时候，FLAGS_use_mkldnn=0，也是能输出的。
There was a problem hiding this comment.
变量use_mkldnn,是与WITH_MKLDNN无关的。这里是加了一个判断，防止用户在WITH_MKLDNN=OFF的时候use_mkldnn=True。其实不加这个，paddle会在register layer type的时候也会有警告，但是觉得那时候FATAL不好，所以加在这里一个。我相信gpu应该哪里也有类似check的。
There was a problem hiding this comment.
用户在WITH_MKLDNN=OFF的时候use_mkldnn=True
这里只是打印，并不需要加ifdef。应该在别的地方做check。不然每次打印的时候，都要加ifdef么？
| if (!inputLayer->hasNextLayer(thisLayer)) { | ||
| inputLayer->addNextLayer(thisLayer); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
- 当前layer不需要知道下一个layer是什么。
- 每个layer的输入和输出是一样的，转换格式应该在layer内部做，用来调用mkldnn库。
- 调用cudnn之前，也没有判断前后layer的情况。
| @@ -105,6 +105,147 @@ class Layer { | |||
| std::vector<std::shared_ptr<FunctionBase>> backward_; | |||
|
|
|||
| public: | |||
| #ifdef PADDLE_USE_MKLDNN | |||
There was a problem hiding this comment.
不会引入太多的逻辑
这段新增的代码有100多行，逻辑已经非常多了。新增库不应该在layer.h中增加逻辑。库的逻辑不应该在这里体现。会给后续维护增加困难。
直接不管输入输出是不是mkldnn，我都可以以默认格式来，那就会有很多没有必要的转换
输入输出应该是paddle默认的格式。调用mkl库的时候，也不管输入输出是不是mkl，为什么mkldnn需要这样呢？
在混用的同时提高性能
可以理解能提高性能，但这部分性能会有多少的提高？现在这样的写法，使得有两套数据结构，后续的维护会非常麻烦。
关于activation，首先在Intelcaffe里面他是独立出来的一个layer，但是在paddle里面的概念不是的（如果我的理解有误，请帮忙指出），所以layer需要的操作对activation实际上也是需要的，这里主要是指resetXXX。然后，关于为什么一定需要这个reset函数，mkldnn里面比较看重的是primitive，他是用来计算的最小单位，但是在使用他之前需要定义好他的各种信息，包括size和format等，一旦定义好了，在使用中是不变的。如果size发生了变化，就需要reset这个primitive，所以才需要这个reset函数，对等在Intelcaffe里面是reshape。所以也就是说，每个mkldnn layer都会支持动态的改变输入size，包括图像大小改变和batchsize的动态改变。
关于这一点，其实还是跟Intelcaffe是类似的，从他里面基本可以看主要长度在setup primitive上，以他的inner product为例就有400多，这还没考虑他在framework内部集成了很多mkldnn的函数接口。其次我把这些setup都流程化了，方面develop在小白的情况下知道是在干什么，并且会使得结构更清晰也更方便debug。 |
Paddle是支持的，forward函数里的resetOutput函数，就是动态改输入size和batchsize的 |
这点我知道，但是clear memory这个会非常耗时，我之前专门关注过这个函数。在mkldnn里面会判断，只在必要的时候才resetouput，可以节省很多时间。 |
指的是置0操作么，paddle的fc用reserveOutput，不置0。直接将输出覆盖。 |
| @@ -36,8 +36,8 @@ include(simd) | |||
| ################################ Configurations ####################################### | |||
| option(WITH_GPU "Compile PaddlePaddle with NVIDIA GPU" ${CUDA_FOUND}) | |||
| option(WITH_AVX "Compile PaddlePaddle with AVX intrinsics" ${AVX_FOUND}) | |||
| option(WITH_MKLDNN "Compile PaddlePaddle with mkl-dnn support." OFF) | |||
| option(WITH_MKLML "Compile PaddlePaddle with mklml package." OFF) | |||
| option(WITH_MKLDNN "Compile PaddlePaddle with mkl-dnn support." ${AVX_FOUND}) | |||
There was a problem hiding this comment.
WITH_MKLDNN为什么是用到AVX_FOUND来设定？
| if (!inputLayer->hasNextLayer(thisLayer)) { | ||
| inputLayer->addNextLayer(thisLayer); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
我也认为当前Layer不需要知道下一个Layer是什么，每个Layer直观按照自己的格式输出就行，转换可以在下一个Layer里面做。另外，比如nchw转mkldnn的内部格式 mkldnn内部是什么样的格式？
| @@ -105,6 +105,147 @@ class Layer { | |||
| std::vector<std::shared_ptr<FunctionBase>> backward_; | |||
|
|
|||
| public: | |||
| #ifdef PADDLE_USE_MKLDNN | |||
There was a problem hiding this comment.
引入'MKLNN'应该是针对一些特殊Layer的修改。
另外，这个PR是enable mkldnn_fc layer，但是这里很多逻辑看起来跟fc layer并没有关系。
| // If restore from this checkpoint must set it true | ||
| // TODO(TJ): always save mkldnn weight as paddle format when checkpoint | ||
| // then can remove this flag | ||
| optional bool init_wgt_from_mkldnn = 57 [default = false]; |
There was a problem hiding this comment.
引入'MKLNN'应该是针对一些特殊Layer的修改。
另外，这个PR是enable mkldnn_fc layer，但是这里很多逻辑看起来跟fc layer并没有关系。
就像前面说一样，这里面添加是基本的功能，在mkldnn layer里面会被调用，并且也只能以父类的方式调用，所以才加功能。
init_wgt_from_mkldnn在 MkldnnLayer.h loadConfig里面被引用
There was a problem hiding this comment.
loadConfig里面赋值给了initWgtFromMkldnn_，但initWgtFromMkldnn_也没见哪有用啊。我看到这个变量有一段注释 compatible initial weight from original cpu weights format，但是还是理解不了这个注释指的是什么意思。original cpu weights format是指类似于nChw8c这样的mkldnn format？
There was a problem hiding this comment.
有一个函数叫initwgtfrompaddle，在mkldnnfc的cpp里面有用的。目的是初始化的时候把paddle的weight转为dnn用的，original cpu weights format指的是paddle原来的weight，不是dnn直接用的.
| << ",cudnn_version=" << hl_get_cudnn_lib_version(); | ||
| #ifndef PADDLE_USE_MKLDNN | ||
| CHECK(!FLAGS_use_mkldnn) << "Can not use mkldnn, please set WITH_MKLDNN=ON"; | ||
| #endif |
There was a problem hiding this comment.
没开启mkldnn的时候，FLAGS_use_mkldnn=0，也是能输出的。
| // TODO(TJ): uncomment me when support multi inputs | ||
| // #ifdef PADDLE_USE_MKLDNN | ||
| // DEFINE_bool(use_mkldnn, true, "Use MKLDNN for training"); | ||
| // #else |
There was a problem hiding this comment.
zeroMem函数，在10个batch之后，基本维持在52s（3.4%），比较稳定。
52s很长了，我现在没有数据，但是我影响中一般一个layer顶多就几百ms左右，zeromem如果在几十ms就很夸张了，并且paddle的zeromem是单core清零的，会很慢。另外reserveOutput还是会清grad吧，dnn其实不需要的，他是覆盖的方式。
There was a problem hiding this comment.
52s这里是笔误，是52ms。一个batch的时间是1500ms左右。52ms是所有layer的zeromem的总和，所以基本可以忽略不计的。
There was a problem hiding this comment.
嗯，所以嘛，我说的是paddle在cpu only的情况。
paddle里面原本的CpuMatrix的zeroMem仅仅是调用memset，性能是单core的性能，是很慢的。
然而在GPU里面用的是gpuMatrix的zeroMem，实际调用的资源多少不是一样的。
| if use_mkldnn: | ||
| config_assert( | ||
| len(inputs) == 1, | ||
| "MkldnnFCLayer support one and only one input!") |
There was a problem hiding this comment.
为什么可以删？我觉得是必要的啊，不是mkldnn的时候，paddle的fc不是可以支持input>1的吗？
There was a problem hiding this comment.
1594的if条件可以和1598的if条件合并成一个。不用做两次判断了。
There was a problem hiding this comment.
哦哦，我原本是以为super的init会对inputs有些处理，所以特意分开了，那就都放在super前面。谢谢。
| config_assert(not sparse, | ||
| "MkldnnFCLayer do not support sparse format yet") | ||
| else: | ||
| dims = [input_layer.size, self.config.size] |
There was a problem hiding this comment.
use_mkldnn后的dims和原来的dims刚好相反了？
| @@ -105,6 +105,147 @@ class Layer { | |||
| std::vector<std::shared_ptr<FunctionBase>> backward_; | |||
|
|
|||
| public: | |||
| #ifdef PADDLE_USE_MKLDNN | |||
There was a problem hiding this comment.
- 两套格式：一套是paddle自己的格式，一套是mkldnn的格式。
- 如果不做转换，其他没有用mkldnn的layer怎么接用了mkldnn的layer。
- 目前写法是判断上下layer是不是mkldnn。在paddle中，layer是不知道下面的layer信息的。paddle后续重构后，op也是不知道后续op的任何信息的。
- 如果要使用这个格式的话，从data layer进来就要转成mkldnn的内部格式，依次往下传递，这样就不需要每次都转换
现在的判断过程只会在mkldnn layer里面才会实际调用，别的layer没有任何影响，父类只是需要提供接口。
关于格式问题， 其实大家关于父类修改的意见我其实是非常同意的，但是我之前已经尝试过很多种方法，在我原来的实现上比这个还要多些。原本我也是想让父类layer不要添加任何东西最好，但是发现如果想要性能，现在这种做法已经是非常简练的了，并且我认为我已经是尽可能少的改动父类了。 如果大家有更加合适的做法，我也是非常欢迎的。 |
我认为应该在这个层面加，layer是不关心底下调用什么库的，它只需要调用抽象的、封装好的函数库。openblas/mkl/cuda都是这样处理的。 看fclayer的代码，非常简单明了，从代码中就能推出数学公式。@hedaoyuan |
|
|
目前没有具体的报告，但是基本上你可以理解为一个转换是一个transpose(也有可能是多个)，随数据量增加，随着layer的层数增加。 |
这个是可以理解的，因为用的都是基本clabs，所以可以体现出fc本身的逻辑，就跟重新实现一个FC一样。 |
但是随着数据量增加和layer层数的增加，总体计算时间也是增加的。这部分的占比是保持不变的。 |
既然和cudnn是类似的，paddle集成cudnn非常干净，像cudnnBatchNormlayer也封装的很好。mkldnn应该也可以做成这样。 |
这是因为还有不止一个辅助文件，比如hl_cuda_cudnn.cc， |
占比是不变的，但是这个占比其实是多余，我原本的考虑是能不要这个多余就不要。 |
hl_batch_norm_forward_training这个函数就只有几十行，这个文件有1100行，但这个文件是所有调用的cudnn函数都写在一块了。现在mkldnnFclayer就620行了。
Paddle当时没考虑把resetOutput去掉的原因是一样的。这个占比不是主要矛盾，集成进framework后不能只考虑性能一方面。 |
|
Let's close this PR. |
enable mkldnn_fc layer
add unit test for mkldnn layers
Feel free @me, if any Inappropriate or questions.