fixed config.mk and Makefile bugs for installing mkl

Description
UPDATES:
I tried difference ways to build mxnet from source by hiding each or both of the two changes I made below, and found that only changing lower case "use_blas" to capital format in Makefile would be able to build mxnet with "BLAS_MKL". The other change, adding "USE_BLAS = mkl" before lines 137 - 142, would not make any impact for this purpose (although I am not sure whether or not this change would make impact at other places).
(Brief description on what this PR is about)
I have found two bugs in the Makefile file and make/config.mk file in apache/incubator-mxnet package. These bugs might lead to incorrect installation of Mxnet with MKL library.
Before the changes, I could not find "BLAS_MKL" in mxnet.runtime.Features() outputs. After rebuilt my mxnet with the following changes, I found "BLAS_MKL" in mxnet.runtime.Features() outputs and the model inference speed I was benchmarking increased by around 35%.
In the make/config.mk file, USE_BLAS will be first set to "atlas" for linux or "apple" for osx by default as follows (lines 119 - 124)
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S), Darwin)
USE_BLAS = apple
else
USE_BLAS = atlas
endif

Later, the USE_STATIC_MKL would be set "NONE" if users did not set USE_BLAS to "mkl" before the following lines (lines 137 - 142). This would not be corrected even if users set "USE_BLAS=mkl" at the end of the config.mk file or in the command line.
# If use MKL only for BLAS, choose static link automatically to allow python wrapper
ifeq ($(USE_BLAS), mkl)
USE_STATIC_MKL = 1
else
USE_STATIC_MKL = NONE
endif

So, I correct this block to the follows,
# If use MKL only for BLAS, choose static link automatically to allow python wrapper
# Please Note: You have to set USE_BLAS = mkl here if you want to build mxnet with mkl. Otherwise USE_STATIC_MKL will be set to NONE.
# USE_BLAS = mkl
ifeq ($(USE_BLAS), mkl)
USE_STATIC_MKL = 1
else
USE_STATIC_MKL = NONE
endif

In the Makefile, lines 247 - 255 as follows, I corrected the "use_blas" to capital form.
ifeq ($(use_blas), open)
	CFLAGS += -DMXNET_USE_BLAS_OPEN=1
else ifeq ($(use_blas), atlas)
	CFLAGS += -DMXNET_USE_BLAS_ATLAS=1
else ifeq ($(use_blas), mkl)
	CFLAGS += -DMXNET_USE_BLAS_MKL=1
else ifeq ($(use_blas), apple)
	CFLAGS += -DMXNET_USE_BLAS_APPLE=1
endif

Checklist
Essentials
Please feel free to remove inapplicable items for your PR.

[ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
[x] Changes are complete (i.e. I finished coding on this PR)
[ ] All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
[ ] Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
[x] To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

[x] In the make/config.mk file, add # USE_BLAS = mkl  and comments for setting correct USE_STATIC_MKL as described in Description section
[x] In the Makefile, lines 247 - 255 as follows, I corrected the "use_blas" to capital form

Comments

This change should be backward compatible.
Don't see any edge cases

@larroy

One question that I have about your description, where do we define USE_STATIC_MKL?  I see it in files define ad-hoc but not in the Makefile or config.mk, could you edit the description and clarify where USE_STATIC_MKL is updated or add a comment in Makefile or config.mk or documentation?  I would expect MKL to be statically linked in this case.

@larroy, USE_STATIC_MKL is updated in config.mk between lines 137 - 142, which I had put in the description.

@TaoLv, Thanks for your suggestion!
After further experiments, I found that changing lower case "use_blas" to capital format in Makefile would be able to build mxnet with "BLAS_MKL". The other change of setting "USE_BLAS=mkl" at different places (e.g. in commad line, at the end of config.mk, or before lines 137 - 142 of config.mk) would not make impact for this purpose.
More specifically, without the changing lower case "use_blas" to capital format in Makefile, adding USE_BLAS=mkl in the make command line would not build mxnet with "BLAS_MKL".
One thing to clarify here is that I used "make" instead of "cmake" to build mxnet from source.

I think the lower cases BLAS check in Makefile was added for runtime feature detection. It's not really used for BLAS linkage. Please correct me if I'm wrong @larroy .
I tried USE_BLAS=mkl in make command line and can find MKL .a files in the link line and there is no MKL .so files in ldd output. Can you double check? @nuslq

Shouldn't USE_STATIC_MKL appear in the Makefile directly then?

@larroy it's passed to and used in mshadow.mk.

@TaoLv yes seems I introduced this, and was not correct due to case. This patch fixes that, I verified on mac and approved the PR, about MKL I think you are the best one to check that part. Since there's no real change to the former I already approved and verified. Thanks for the fix!

@TaoLv,
I built mxnet using command line "make -j $(nproc) USE_BLAS=mkl", and got the followings from "ldd lib/libmxnet.so | grep mkl",
libmkldnn.so.0 => /home/ubuntu/incubator-mxnet/lib/libmkldnn.so.0
libmklml_intel.so => /home/ubuntu/incubator-mxnet/lib/libmklml_intel.so

Thank you for confirming. @larroy
@nuslq,  this is as expected. libmkldnn.so and libmklml_intel.so will be dynamically linked even USE_STATIC_MKL is true. Actually, they are not the MKL in USE_STATIC_MKL. Please refer to the logic here: https://github.com/dmlc/mshadow/blob/master/make/mshadow.mk#L87

USE_BLAS=mkl and USE_STATIC_MKL are for how to link MKL BLAS. They can be used even USE_MKLDNN=0 (in this case, you will not see the mkldnn.so and mklml_intel.so in ldd).
mklml_intel.so is introduced along with USE_MKLDNN=1 to improve the performance of mkldnn.so. mklml_intel cannot be statically linked as no .a file is provided.

Hope this explanation can address your questions. So the fix for lower case use_blas looks good. But the change and description for USE_STATIC_MKL does not make sense to me.

@mxnet-label-bot add [MKL, installation]

Thanks for the explanation @TaoLv. Here are some questions inlined.

@nuslq, this is as expected. libmkldnn.so and libmklml_intel.so will be dynamically linked even USE_STATIC_MKL is true. Actually, they are not the MKL in USE_STATIC_MKL. Please refer to the logic here: https://github.com/dmlc/mshadow/blob/master/make/mshadow.mk#L87

As far as we can trace, the only instance where the MKL usage flag, MSHADOW_USE_MKL is set in CMake files but not in Makefile. Could you confirm that?

USE_BLAS=mkl and USE_STATIC_MKL are for how to link MKL BLAS. They can be used even USE_MKLDNN=0 (in this case, you will not see the mkldnn.so and mklml_intel.so in ldd).

mklml_intel.so is introduced along with USE_MKLDNN=1 to improve the performance of mkldnn.so. mklml_intel cannot be statically linked as no .a file is provided.

Hope this explanation can address your questions. So the fix for lower case use_blas looks good. But the change and description for USE_STATIC_MKL does not make sense to me.

I feel like there is more to understand which flags (as compiler and linker flags) are absolutely necessary for MxNet to compile with MKL BLAS support, using make or cmake. Moreover, I cannot find any place in mshadow code the compiler definition  MSHADOW_USE_MKL, but only in MxNet operators, which is never set with Makefile (as far as I can trace). This was one of the reasons we wanted to remark on USE_STATIC_MKL since we couldn't find the aforementioned compiler definition set.
Could you or @larroy clarify what we're missing here?

@yugoren Please find the definition of MSHADOW_USE_MKL at  https://github.com/dmlc/mshadow/blob/master/mshadow/base.h#L88 and https://github.com/dmlc/mshadow/blob/master/make/mshadow.mk#L101

@yugoren Please find the definition of MSHADOW_USE_MKL at https://github.com/dmlc/mshadow/blob/master/mshadow/base.h#L88 and https://github.com/dmlc/mshadow/blob/master/make/mshadow.mk#L101

Thanks, that clears out how MKL is enabled by default in mshadow!  Also, going back to the initial concern @TaoLv had about the comment we added: even if we pass USE_BLAS=mkl to make, the lines that @nuslq pointed out overrides the variable
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S), Darwin)
USE_BLAS = apple
else
USE_BLAS = atlas
endif

That's why we felt it would be informative for the user to know exactly where to set the flag. Maybe we can change it to the following
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S), Darwin)
USE_BLAS ?= apple
else
USE_BLAS ?= atlas
endif

@larroy What do you think?
Another question: is MSHADOW_USE_MKL enabled in mxnet build when mshadow/base.h is included in the files (maybe cascaded)? Could you also clarify why the flag is clearly set to 1 in cmake while set only through including a header file in make? This was one of the reason it was hard for me to track down how the flag is enabled in mxnet side and I was considering adding it into compiler definitions, probably in a separate PR.

Indeed this is a mess and we should improve it. I suggest having this PR merged if anyone doesn't have any additional concerns to credit the contribution and adding additional PRs to improve the situation by clearly specifying the BLAS related flags in the compilation files and documenting them.

@yugoren Can you double check? I can statically link MKL BLAS library by adding USE_BLAS=mkl to the make command line.

@yugoren Can you double check? I can statically link MKL BLAS library by adding USE_BLAS=mkl to the make command line.

@TaoLv, Yes, we can statically link MKL BLAS library by adding USE_BLAS=mkl to the make command line. I have removed the comments for USE_BLAS=mkl in this PR.

@nuslq thank you for the turn around. Approve now.
It would be better if you can also change the description of this PR:

This would not be corrected even if users set "USE_BLAS=mkl" at the end of the config.mk file or in the command line.

@TaoLv, updated the description

@nuslq Thank you for the changes. It's now merged.


A few changes to support Base64 strings properly. The code was also formatted.

Hello,
I ran the crate through cargo fmt.
I also fixed a small problem with Base64 strings, where '=' and '+' where creating an EncodeError.

This should solve the issue #5.

Thanks for the PR! I agree that the code should be eventually reformatted, but I'd prefer to wait for the formatting RFC process to run its course (it's not too far off).
As for the Base64 encoding/decoding, the change you've proposed would accept invalid encodings, since bcrypt explicitly doesn't include + and = in its Base64 set (which is different from the set used by MIME). See the encoding string: in addition to a-zA-Z0-9, . and / are used to round it off.
So, I can't accept the PR as is -- but you can open an issue requesting rustfmt, which can be accompanied by a new PR once there's a fully defined set of formatting rules.

The problem is that the crate base64 generates Base64 strings based on the MIME one. The data encoding crate bases its implementation on RFC4648 which also includes + and =.
How do you generate your salts on your side ?

I don't see why bcrypt uses an other set that the one used by MIME

The + has special meaning in the Unix password file when the historic NIS/YP resolution is active. While it's unlikely that a salt or hash value would end there, it was felt that any chance of confusion would be best eliminated by using something else. Likewise, = separates variables and values in the shell.
I haven't tried to find the authoritative source for the encoding scheme; my code is basically a port of one of the two canonical C implementations, the Openwall one.

How do you generate your salts on your side ?

Internally, with random::gen_salt_bytes(). If you call bcrypt::hash() with just the cleartext password, a random salt will be generated for you.

Okay thanks, I'll make a pull request with a bit more documentation on this point !


[Bug]

Search before asking

[X] 我在 issues 列表中搜索，没有找到类似的内容。
I searched in the issues and found nothing similar.

Pake version
latest
System version
Macos
Node.js version
latest
Minimal reproduce step
Wiki中的文档Online-Compilation-(used-by-ordinary-users).md

这里url有错误打开404
_https://github.com/tw93/Pake/blob/master/bin/README_EN.md#usage_
要改成 https://github.com/tw93/Pake/blob/master/bin/README.md#usage
What did you expect to see?
link valid
What did you see instead?
link invalid
Anything else?
No response
Are you willing to submit a PR?

[X] I'm willing to submit a PR!

已修复，感谢提醒。


Will you add a browser to ToaruOS?

?

ToaruOS has a browser, it's just a bit hidden as it's part of the help document viewer as its HTML rendering is not quite complete enough to be useful to most people:

There's always interest in porting a more complete third-party rendering engine, such as Webkit, but this is a monumental task.

It skips CSS styles as I can see
What about Gecko? I know that it currently runs a like a **** and consumes a lot of RAM unnecesary but I still hope they'll fix it someday

Webkit is probably an easier port as it's seen a lot more platforms, but I would love Gecko. Either way, modern rendering engines are very complex beasts. We might do well to look at some very old versions of Gecko, Webkit, and KHTML to see if they are more suitable for porting. Meanwhile, I'm busy with another project at the moment.

I recently compared performance of Firefox 6.01 with latest Nightly and it was drastic
FF6 is flying with 30+ open at once and consuming 500 Mb of RAM
(But some features and stylings are, of course, broken)
In same conditions Nightly would probably freeze and consume the whole 1 Gb
My recent idea was to take that FF6 codebase and provide some modern features support

@BlackPhrase Therein lies the problem. Those modern features account for most of the performance losses. Sure, there generally exists a tradeoff between speed and memory consumption in most software problems but it is necessary to choose either one or the other.

Thanks


DataGrid Horizontal Scrolling Bug (?)

Looks like a minor bug:
I have a DataGrid with many columns. So i need to horizontally scroll to see all data.
While scrolling i get in the output window :
System.Windows.Data Error: 5 : Value produced by BindingExpression is not valid for target property. Double:'-0.028838485341793785' BindingExpression:Path=CellsPanelHorizontalOffset; DataItem='DataGrid' (Name=''); target element is 'Button' (Name=''); target property is 'Width' (type 'Double')
Also the scrolling is not smooth as expected.
The problem looks to be on "AllGeometry" Button (DataGrid.xaml). Changing the binding on Width property to let's say "50", works as expected.
Thanks.

@ghost1372 Can you reproduce the problem?

hi @NaBian  I tried,  the Datagrid seems to have a problem
The following image is taken along with the following settings as you can see that although the width of the items is specified But the items are stuck together
HorizontalScrollBarVisibility="Visible"
RowHeaderWidth="130"
<DataGridTextColumn Width="100"

Also, when we do not specify the width, even with the scroll bar, some items cannot be displayed
In the specified section we should have checkbox and the Combobox which is not

Let me also say that scrolling with 20 rows and 50 or 30 columns is slow and laggy And the other problem that exists is that you have to wait for the data to load so that the scroll bar appears
I know that 30 columns is not rational, but Should be considered
another bug is when you set width=50 with 40 columns you get this

you can test with this code in HandyControlDemo ----> DatagridDemoCtl
               <DataGrid HorizontalScrollBarVisibility="Visible" HeadersVisibility="All" RowHeaderWidth="130" AutoGenerateColumns="False" ItemsSource="{Binding DataList}">                     <DataGrid.RowHeaderTemplate>                         <DataTemplate>                             <CheckBox IsChecked="{Binding IsSelected,RelativeSource={RelativeSource AncestorType=DataGridRow}}"/>                         </DataTemplate>                     </DataGrid.RowHeaderTemplate>                     <DataGrid.Columns>                         <DataGridTextColumn IsReadOnly="True" Width="80" CanUserResize="False" Binding="{Binding Index}" Header="{x:Static langs:Lang.Index}"/>                         <DataGridTemplateColumn Width="60" CanUserResize="False">                             <DataGridTemplateColumn.CellTemplate>                                 <DataTemplate>                                     <Image Source="{Binding ImgPath}" Width="32" Height="32" Stretch="Uniform"/>                                 </DataTemplate>                             </DataGridTemplateColumn.CellTemplate>                         </DataGridTemplateColumn>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridTextColumn Binding="{Binding Name}" Header="{x:Static langs:Lang.Name}"/>                         <DataGridCheckBoxColumn Width="100" CanUserResize="False" Binding="{Binding IsSelected}" Header="{x:Static langs:Lang.Selected}"/>                         <DataGridComboBoxColumn ItemsSource="{Binding Source={StaticResource DemoTypes}}" Width="100" CanUserResize="False" SelectedValueBinding="{Binding Type}" Header="{x:Static langs:Lang.Type}"/>                         <DataGridTextColumn Width="1*" Binding="{Binding Remark}" Header="{x:Static langs:Lang.Remark}"/>                     </DataGrid.Columns>                 </DataGrid>

hi @metheofanis i didnt see this error
System.Windows.Data Error: 5 : Value produced by BindingExpression is not valid for target property. Double:'-0.028838485341793785' BindingExpression:Path=CellsPanelHorizontalOffset; DataItem='DataGrid' (Name=''); target element is 'Button' (Name=''); target property is 'Width' (type 'Double')
Can you provide more information? (Number of columns and their width)
An example project can be great if possible

Hi @ghost1372 ,
It looks like I get the error in any number of columns/rows! I had 21 columns. I removed all and kept only two.
I resized the window to make it very small, so that horizontal scrolling was required. I got that problem. I've changed the width from auto to fix 50 or 100. Same problem.
I had more than 4000 rows. I changed it to get only 10 rows, the same problem.
As I told before: The problem looks to be on "AllGeometry" Button (DataGrid.xaml). Changing the binding on Width property to let's say "50", works as expected. Kindly note: I change this in HandyControl's DataGrid.xaml , not mine!
I'll try to make a small project to show this soon.
Regards. :)

remove this:

@NaBian  tnx problem fixed 😁 The only problem now is that scrolling is not smooth And if there are more records, And using the SelectAllButton takes a long time for all records to be selected
@metheofanis   I tested with 4,000 records and more than 40 columns with a width of 100 and didn't get the error you said

I got it!
I had no: RowHeaderWidth="60"
After adding it, the problem has gone. Maybe if you remove it, you could see what I mean.
I'll investigate more tomorrow!
Thank a lot!

@metheofanis  I removed but I didn't see any error again

Here I'm trying to insert an animated gif. First time. Please excuse if not a success:
Image Here

@metheofanis  i think link is broken

What about this?

Hello,
Here is a new test on a new project. 1 Column, 1 Row. The column MinWidth is set to a greater value than the window, just to show the scrollbar. Below the image is the project in a zipped format

HandyControlTest2.zip

@NaBian
@metheofanis
if we specify the RowHeaderTemplate and specify the HeadersVisibility to All we will not get the error but if these two are not defined we will get an error
@metheofanis  for fixing problem you can put this code in datagrid
<DataGrid.RowHeaderTemplate>
                        <DataTemplate>
                            <CheckBox IsChecked="{Binding IsSelected,RelativeSource={RelativeSource AncestorType=DataGridRow}}"/>
                        </DataTemplate>
                    </DataGrid.RowHeaderTemplate>

Working!
Thanks a lot.👍


Application logic invariant error

The query-engine allows invalid data to be inserted.
Concretely: While model A has a required relation to model B, query-engine allows inserting a model A record, without providing B.
Querying that data then fails.
Full Repro: https://github.com/timsuchanek/prisma-engine-255-repro
It will either end up in
Application logic invariant error: received null value for field blog which may not be null in query-engine/core/src/response_ir/internal.rs:228:26

Awaiting confirmation that this is still an issue.

Confirmed still an issue but with different error now. I will proceed to check if this error is sufficient (it's originating from the connector), or if we should throw a different, more descriptive error.

Considered a sufficiently descriptive error with Preview-17. Closing this issue.