You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/02/10 15:19:13 UTC
[GitHub] [incubator-mxnet] kpuatamazon opened a new pull request #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
kpuatamazon opened a new pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559
## Description ##
This pull request adds wrappers to the intgemm matrix multiplication library: https://github.com/kpu/intgemm .
A performance comparison with DNNL aka MKL-DNN is at https://github.com/kpu/intgemm/issues/59
The library targets thin matrix sizes seen in neural machine translation inference and was part of the top submission to the 2018 Workshop on Neural Generation and Translation efficiency task: https://neural.mt/papers/edinburgh/wnmt_marian_paper.pdf . The purpose of this issue is to add similar functionality to Sockeye: https://github.com/awslabs/sockeye/pull/771 .
Quantized Sockeye performance is 2.95x as fast. One problem with the current MXQuantizeSymbol approach is that Sockeye does not have a static graph for everything.
intgemm uses a custom memory layout for the weight matrix to make more memory accesses consecutive, so there are operators to convert weights to that format. The idea is that weights are typically loaded once for inference.
On architectures without VNNI, intgemm uses saturating 16-bit accumulation. This avoids an expensive madd_epi16 instruction every multiply by exploiting the fact that most neural network parameters are near 0.
Because x86 only offers a unsigned * signed instruction and most people want signed * signed, there are two strategies one can take.
Add 128 to data so now it's unsigned. But that biases the output. DNNL calculates this bias on the fly by summing weights then subtracts it out during GEMM. intgemm calculates this bias in advance, which can then be subtracted from the bias term with no overhead at runtime. A problem with this strategy is that it makes the accumulator bigger, requiring more upcasting with an expensive madd_epi16 instruction.
Emulate signed * signed by normalizing the sign bit into the second argument. This requires extra instructions in the hot loop but keeps the accumulator small, so it's less necessary to accumulate into 32-bit integers and madd_epi16 can be avoided.
Both intgemm and DNNL implement strategy 1; intgemm also implements strategy 2.
Similar to DNNL, intgemm has runtime CPUID selection among backends for SSSE3, AVX2, AVX512BW, and AVX512VNNI.
## Checklist ##
### Essentials ###
Please feel free to remove inapplicable items for your PR.
- [x] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) 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 https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
- [x] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
### Changes ###
- [x] submodule for intgemm
- [x] `intgemm_prepare_data` and `intgemm_prepare_weight` operators to convert operands from fp32
- [x] `intgemm_take_weight` for taking weights in intgemm's weight format, which is useful for vocabulary shortlists in Sockeye.
- [x] `intgemm_fully_connected` for matrix multiply
## Comments ##
Backward compatible.
intgemm requires the inner dimension be a multiple of 64 for efficiency and alignment reasons. Currently the outputs must be a multiple of 8 but there is in-progress code in intgemm to remove that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590256391
@pengzhao-intel Which OMP do you recommend? I've been getting bad OMP results with a stock install of Ubuntu 18, but am happy to sprinkle a `#pragma omp parallel for` on the embarrassingly parallel quantization task and add a loop at the end to deal with non-multiples of register size.
Also, how many threads was the matrix benchmark run on? All of them?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
pengzhao-intel edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590146883
@kpuatamazon We have tested the performance in our local machine pre- and VNNI feature for both data conversion and computation part (FC layer).
Thanks, @ElaineBao's work on data measurements.
In general, the intgemm can provide better performance on data conversion with less core (1 core) and this may help on client CPU. However, the computation parts and data conversion with more cores performance are still slower than the current integration. Meanwhile, the supported shape is only limited in number dividable by 4.
**detail data as below**
- FC layer runtime from current MXNet OP with intgemm by 28 cores in 1 socket.
Note: the smaller the better in below table
Thus, the current MXNet OP is still faster in most cases for two-generation CPU.
|CLX8280 VNNI (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.08 | 0.09 | 0.10 | 0.11 | 0.12
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.45 | 0.86 | 1.71
|SKX8180 (non VNNI) (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.07 | 0.09 | 0.11 | 0.13 | 0.15
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.46 | 0.90 | 1.78
- Data conversion from FP32 to INT8
1 core results show the intgemm has the benefit on data conversion.
shape | quantize_v2 | intgemm | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | -- | -- | --
(128, 128) | 0.062 | 0.0224 | 0.0675 | 0.0442 | 1.5x
(256, 256) | 0.0794 | 0.026 | 0.1264 | 0.0577 | 2.2x
(512, 512) | 0.1615 | 0.0863 | 0.4025 | 0.1757 | 2.3x
(1024, 1024) | 0.3422 | 0.2485 | 1.3097 | 0.5153 | 2.5x
(2048, 2048) | 1.0238 | 0.8415 | 4.7436 | 1.7523 | 2.7x
(8, 4096) | 0.0666 | 0.0235 | 0.0845 | 0.0485 | 1.7x
(4096, 8) | 0.0657 | 0.0236 | 0.0841 | 0.0482 | 1.7x
But with more cores (28 cores), the current OP is slight better.
shape | quantize_v2 | intgemm | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | -- | -- | --
(128, 128) | 0.0757 | 0.0356 | 0.0767 | 0.0704 | 1.1x
(256, 256) | 0.0773 | 0.0408 | 0.084 | 0.0907 | 0.9x
(512, 512) | 0.0798 | 0.0949 | 0.1083 | 0.2163 | 0.5x
(1024, 1024) | 0.0892 | 0.244 | 0.1645 | 0.5415 | 0.3x
(2048, 2048) | 0.1441 | 0.76 | 0.3507 | 1.769 | 0.2x
(8, 4096) | 0.0752 | 0.037 | 0.0798 | 0.0766 | 1.0x
(4096, 8) | 0.0755 | 0.0372 | 0.0795 | 0.0771 | 1.0x
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on issue #17559: [MXNET-1446]
Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
ciyongch commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-591207931
> The `prepare_data` step is just a quantizer. The `prepare_weight` step does some element rearragement into a CPU-dependent format so the API isn't quite the same as the current one.
> And bias works in the usual way. Internally intgemm takes a template argument for what to do with the int32 output while it's still in a register. So one can write out floats or apply activation functions in registers first.
@kpuatamazon Actually, MXNet use a unified quantization flow (please refer to `quantize_model` from [here](https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/contrib/quantization.py)) to do the quantization for activation/weights, and invoke quantized Ops(quantized_conv/quantized_fc/quantized_relu, etc.) when they're available. But the current implementation works in a different way, which requires user to modify their models to call intgemm API directly and not that friendly for either fallback (to FP32) or accuracy tuning.
> The multiple of 8 issue is relatively easy to fix; that was just us being lazy about smaller tiles. Having inner dimension a multiple of 64 is rather useful for alignment purposes; how does MXNet deal with padding?
MXNet framework relies BLAS lib or DNNL lib to handle such cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590265641
@ciyongch Here is a usage example.
```python
import mxnet as mx
#This is done offline.
weight = mx.nd.random_uniform(shape=(8,64), low=-1.0, high=1.0)
weight_max = mx.nd.contrib.intgemm_maxabsolute(weight)
weight_prepared = mx.nd.contrib.intgemm_prepare_weight(weight, weight_max)
data = mx.nd.random_uniform(shape=(1,64), low=-1.0, high=1.0)
#Fused multiply quantizes on the fly.
product1 = mx.nd.contrib.intgemm_fully_connected(data, weight_prepared, scaling = weight_max / 127.0, num_hidden = 8, no_bias = True)
#One can also have quantized data.
data_max = mx.nd.contrib.intgemm_maxabsolute(data)
data_prepared = mx.nd.contrib.intgemm_prepare_data(data, data_max)
product2 = mx.nd.contrib.intgemm_fully_connected(data_prepared, weight_prepared, scaling = weight_max / 127.0 * data_max / 127.0, num_hidden = 8, no_bias = True)
baseline = mx.nd.FullyConnected(data, weight, num_hidden = 8, no_bias = True)
```
The `prepare_data` step is just a quantizer. The `prepare_weight` step does some element rearragement into a CPU-dependent format so the API isn't quite the same as the current one.
And bias works in the usual way. Internally intgemm takes a template argument for what to do with the int32 output while it's still in a register. So one can write out floats or apply activation functions in registers first.
The multiple of 8 issue is relatively easy to fix; that was just us being lazy about smaller tiles. Having inner dimension a multiple of 64 is rather useful for alignment purposes; how does MXNet deal with padding?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587073135
Also, OMP performance in MXNet quantizer is very bad. (NB: intgemm is running single-threaded here, partly because OMP is bad at this problem)
```bash
export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=2; taskset --cpu-list 0,1 ./quant_bench.py
```
```
[16:18:40] src/engine/engine.cc:55: MXNet start using engine: NaiveEngine
Shape (128, 128)
0.0008789 seconds for quantize
0.0008693 seconds for quantize_v2
0.0000175 seconds for intgemm
intgemm is 49.7x faster
Shape (256, 256)
0.0034812 seconds for quantize
0.0034044 seconds for quantize_v2
0.0000212 seconds for intgemm
intgemm is 161.0x faster
Shape (512, 512)
0.0138909 seconds for quantize
0.0138283 seconds for quantize_v2
0.0000731 seconds for intgemm
intgemm is 189.3x faster
Shape (1024, 1024)
0.0557616 seconds for quantize
0.0553598 seconds for quantize_v2
0.0002330 seconds for intgemm
intgemm is 237.6x faster
Shape (2048, 2048)
0.2225617 seconds for quantize
0.2196410 seconds for quantize_v2
0.0008387 seconds for intgemm
intgemm is 261.9x faster
Shape (8, 4096)
0.0017372 seconds for quantize
0.0017434 seconds for quantize_v2
0.0000183 seconds for intgemm
intgemm is 94.8x faster
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on issue #17559: [MXNET-1446]
Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
ciyongch commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587266270
@kpuatamazon Does this new wrappers work in the same way as current MXNet quantization flow? Do you have any simple case to show the typical usage of it?
The current performance number sounds a good achievement with the given shape, can you share the status and plan to the shapes which are not 64 or multiple of 8?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
pengzhao-intel edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590146883
@kpuatamazon We have tested the performance in our local machine pre- and VNNI feature for both data conversion and computation part (FC layer).
Thanks, @ElaineBao's work on data measurements.
In general, the intgemm can provide better performance on data conversion with less core (1 core) and this may help on client CPU. However, the computation parts and data conversion with more cores performance are still slower than the current integration. Meanwhile, the supported shape is only limited in number dividable by 4.
**detail data as below**
- FC layer runtime from current MXNet OP with intgemm by 28 cores in 1 socket.
Note: the smaller the better in below table
Thus, the current MXNet OP is still faster in most cases for two-generation CPU.
|CLX8280 VNNI (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.08 | 0.09 | 0.10 | 0.11 | 0.12
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.45 | 0.86 | 1.71
|SKX8180 (non VNNI) (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.07 | 0.09 | 0.11 | 0.13 | 0.15
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.46 | 0.90 | 1.78
- Data conversion from FP32 to INT8
1 core results show the intgemm has the benefit on data conversion.
shape | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | --
(128, 128) | 0.0675 | 0.0442 | 1.5x
(256, 256) | 0.1264 | 0.0577 | 2.2x
(512, 512) | 0.4025 | 0.1757 | 2.3x
(1024, 1024) | 1.3097 | 0.5153 | 2.5x
(2048, 2048) | 4.7436 | 1.7523 | 2.7x
(8, 4096) | 0.0845 | 0.0485 | 1.7x
(4096, 8) | 0.0841 | 0.0482 | 1.7x
But with more cores (28 cores), the current OP is slight better.
shape | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | --
(128, 128) | 0.0767 | 0.0704 | 1.1x
(256, 256) | 0.084 | 0.0907 | 0.9x
(512, 512) | 0.1083 | 0.2163 | 0.5x
(1024, 1024) | 0.1645 | 0.5415 | 0.3x
(2048, 2048) | 0.3507 | 1.769 | 0.2x
(8, 4096) | 0.0798 | 0.0766 | 1.0x
(4096, 8) | 0.0795 | 0.0771 | 1.0x
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587073135
Also, OMP scaling in MXNet quantizer is not great. (Baseline here is intgemm running single)
`export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=2; taskset --cpu-list 0,1 ./quant_bench.py`
```
Shape (128, 128)
0.0000925 seconds for quantize
0.0001086 seconds for quantize_v2
0.0000181 seconds for intgemm
0.0001093 seconds for quantize_v2_fit
0.0000357 seconds for intgemm_fit
intgemm is 5.1x faster with calibration
intgemm is 3.1x faster without calibration
Shape (256, 256)
0.0001007 seconds for quantize
0.0001192 seconds for quantize_v2
0.0000222 seconds for intgemm
0.0001536 seconds for quantize_v2_fit
0.0000497 seconds for intgemm_fit
intgemm is 4.5x faster with calibration
intgemm is 3.1x faster without calibration
Shape (512, 512)
0.0001490 seconds for quantize
0.0001629 seconds for quantize_v2
0.0000769 seconds for intgemm
0.0003444 seconds for quantize_v2_fit
0.0001610 seconds for intgemm_fit
intgemm is 1.9x faster with calibration
intgemm is 2.1x faster without calibration
Shape (1024, 1024)
0.0002579 seconds for quantize
0.0002620 seconds for quantize_v2
0.0002375 seconds for intgemm
0.0010501 seconds for quantize_v2_fit
0.0004887 seconds for intgemm_fit
intgemm is 1.1x faster with calibration
intgemm is 2.1x faster without calibration
Shape (2048, 2048)
0.0006628 seconds for quantize
0.0006300 seconds for quantize_v2
0.0008444 seconds for intgemm
0.0037701 seconds for quantize_v2_fit
0.0017605 seconds for intgemm_fit
intgemm is 0.7x faster with calibration
intgemm is 2.1x faster without calibration
Shape (8, 4096)
0.0000963 seconds for quantize
0.0001119 seconds for quantize_v2
0.0000188 seconds for intgemm
0.0001231 seconds for quantize_v2_fit
0.0000404 seconds for intgemm_fit
intgemm is 5.1x faster with calibration
intgemm is 3.0x faster without calibration
Shape (4096, 8)
0.0000955 seconds for quantize
0.0001110 seconds for quantize_v2
0.0000191 seconds for intgemm
0.0001237 seconds for quantize_v2_fit
0.0000403 seconds for intgemm_fit
intgemm is 5.0x faster with calibration
intgemm is 3.1x faster without calibration
```
`export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=48; ./quant_bench.py`
```
Shape (128, 128)
0.0002489 seconds for quantize
0.0002599 seconds for quantize_v2
0.0000313 seconds for intgemm
0.0002602 seconds for quantize_v2_fit
0.0000610 seconds for intgemm_fit
intgemm is 7.9x faster with calibration
intgemm is 4.3x faster without calibration
Shape (256, 256)
0.0002440 seconds for quantize
0.0002617 seconds for quantize_v2
0.0000374 seconds for intgemm
0.0002714 seconds for quantize_v2_fit
0.0000813 seconds for intgemm_fit
intgemm is 6.5x faster with calibration
intgemm is 3.3x faster without calibration
Shape (512, 512)
0.0002484 seconds for quantize
0.0002652 seconds for quantize_v2
0.0001080 seconds for intgemm
0.0002987 seconds for quantize_v2_fit
0.0002220 seconds for intgemm_fit
intgemm is 2.3x faster with calibration
intgemm is 1.3x faster without calibration
Shape (1024, 1024)
0.0002648 seconds for quantize
0.0002833 seconds for quantize_v2
0.0002505 seconds for intgemm
0.0003843 seconds for quantize_v2_fit
0.0005266 seconds for intgemm_fit
intgemm is 1.1x faster with calibration
intgemm is 0.7x faster without calibration
Shape (2048, 2048)
0.0003306 seconds for quantize
0.0003467 seconds for quantize_v2
0.0007946 seconds for intgemm
0.0006679 seconds for quantize_v2_fit
0.0017266 seconds for intgemm_fit
intgemm is 0.4x faster with calibration
intgemm is 0.4x faster without calibration
Shape (8, 4096)
0.0002452 seconds for quantize
0.0002593 seconds for quantize_v2
0.0000331 seconds for intgemm
0.0002629 seconds for quantize_v2_fit
0.0000678 seconds for intgemm_fit
intgemm is 7.4x faster with calibration
intgemm is 3.9x faster without calibration
Shape (4096, 8)
0.0002426 seconds for quantize
0.0002600 seconds for quantize_v2
0.0000328 seconds for intgemm
0.0002641 seconds for quantize_v2_fit
0.0000678 seconds for intgemm_fit
intgemm is 7.4x faster with calibration
intgemm is 3.9x faster without calibration
```
`export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=24; ./quant_bench.py`
```
Shape (128, 128)
0.0001447 seconds for quantize
0.0001602 seconds for quantize_v2
0.0000227 seconds for intgemm
0.0001592 seconds for quantize_v2_fit
0.0000458 seconds for intgemm_fit
intgemm is 6.4x faster with calibration
intgemm is 3.5x faster without calibration
Shape (256, 256)
0.0001453 seconds for quantize
0.0001625 seconds for quantize_v2
0.0000275 seconds for intgemm
0.0001685 seconds for quantize_v2_fit
0.0000632 seconds for intgemm_fit
intgemm is 5.3x faster with calibration
intgemm is 2.7x faster without calibration
Shape (512, 512)
0.0001485 seconds for quantize
0.0001654 seconds for quantize_v2
0.0000870 seconds for intgemm
0.0001898 seconds for quantize_v2_fit
0.0001948 seconds for intgemm_fit
intgemm is 1.7x faster with calibration
intgemm is 1.0x faster without calibration
Shape (1024, 1024)
0.0001645 seconds for quantize
0.0001821 seconds for quantize_v2
0.0002382 seconds for intgemm
0.0002562 seconds for quantize_v2_fit
0.0005137 seconds for intgemm_fit
intgemm is 0.7x faster with calibration
intgemm is 0.5x faster without calibration
Shape (2048, 2048)
0.0002296 seconds for quantize
0.0002436 seconds for quantize_v2
0.0008252 seconds for intgemm
0.0005362 seconds for quantize_v2_fit
0.0017207 seconds for intgemm_fit
intgemm is 0.3x faster with calibration
intgemm is 0.3x faster without calibration
Shape (8, 4096)
0.0001451 seconds for quantize
0.0001608 seconds for quantize_v2
0.0000242 seconds for intgemm
0.0001615 seconds for quantize_v2_fit
0.0000510 seconds for intgemm_fit
intgemm is 6.0x faster with calibration
intgemm is 3.2x faster without calibration
Shape (4096, 8)
0.0001451 seconds for quantize
0.0001609 seconds for quantize_v2
0.0000243 seconds for intgemm
0.0001632 seconds for quantize_v2_fit
0.0000511 seconds for intgemm_fit
intgemm is 6.0x faster with calibration
intgemm is 3.2x faster without calibration
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
pengzhao-intel commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587247100
Thanks for the performance data and script.
@ElaineBao could you take a try on both pre-VNNI and VNNI machine?
cc @ZhennanQin
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587073135
Also, OMP performance is very bad. (NB: intgemm is running single-threaded here, partly because OMP is bad at this problem)
```bash
export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=2; taskset --cpu-list 0,1 ./quant_bench.py
```
```
[16:18:40] src/engine/engine.cc:55: MXNet start using engine: NaiveEngine
Shape (128, 128)
0.0008789 seconds for quantize
0.0008693 seconds for quantize_v2
0.0000175 seconds for intgemm
intgemm is 49.7x faster
Shape (256, 256)
0.0034812 seconds for quantize
0.0034044 seconds for quantize_v2
0.0000212 seconds for intgemm
intgemm is 161.0x faster
Shape (512, 512)
0.0138909 seconds for quantize
0.0138283 seconds for quantize_v2
0.0000731 seconds for intgemm
intgemm is 189.3x faster
Shape (1024, 1024)
0.0557616 seconds for quantize
0.0553598 seconds for quantize_v2
0.0002330 seconds for intgemm
intgemm is 237.6x faster
Shape (2048, 2048)
0.2225617 seconds for quantize
0.2196410 seconds for quantize_v2
0.0008387 seconds for intgemm
intgemm is 261.9x faster
Shape (8, 4096)
0.0017372 seconds for quantize
0.0017434 seconds for quantize_v2
0.0000183 seconds for intgemm
intgemm is 94.8x faster
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587066870
The current MXNet quantizer is 3-10x slower than intgemm's on a wide variety of matrix sizes.
Experiment on c5.12xlarge:
```
Shape (128, 128)
0.0000731 seconds for quantize
0.0000706 seconds for quantize_v2
0.0000219 seconds for intgemm
intgemm is 3.2x faster
Shape (256, 256)
0.0002116 seconds for quantize
0.0001778 seconds for quantize_v2
0.0000258 seconds for intgemm
intgemm is 6.9x faster
Shape (512, 512)
0.0008112 seconds for quantize
0.0006480 seconds for quantize_v2
0.0000917 seconds for intgemm
intgemm is 7.1x faster
Shape (1024, 1024)
0.0030176 seconds for quantize
0.0023387 seconds for quantize_v2
0.0002542 seconds for intgemm
intgemm is 9.2x faster
Shape (2048, 2048)
0.0118271 seconds for quantize
0.0090704 seconds for quantize_v2
0.0008705 seconds for intgemm
intgemm is 10.4x faster
Shape (8, 4096)
0.0001187 seconds for quantize
0.0001061 seconds for quantize_v2
0.0000226 seconds for intgemm
intgemm is 4.7x faster
```
Generated by `export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=1; taskset --cpu-list 0 ./quant_bench.py`
where `quant_bench.py` is:
```
#!/usr/bin/env python3
import mxnet as mx
import time
def time_procedure(shape, count, proc):
data = mx.nd.random_uniform(shape=s, low=-1.0, high = 1.0)
mx.nd.waitall()
begin = time.time()
for i in range(0, count):
proc(data)
mx.nd.waitall()
return (time.time() - begin) / count
shapes = [(128, 128), (256,256), (512, 512), (1024, 1024), (2048, 2048)]
count = 1000
one = mx.nd.ones(shape=(1))
minusone = -one
procedures = {
"quantize" : (lambda data : mx.nd.contrib.quantize(data, minusone, one)),
"quantize_v2" : (lambda data : mx.nd.contrib.quantize_v2(data, min_calib_range = -1.0, max_calib_range = 1.0)),
"intgemm" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, one))
}
for s in shapes:
print("Shape " + str(s))
stats = {}
for name, l in procedures.items():
stats[name] = time_procedure(s, count, l)
print("{:.7f} seconds for {}".format(stats[name], name))
best_baseline = min(stats["quantize"], stats["quantize_v2"])
ratio = best_baseline / stats["intgemm"]
print("intgemm is {:.1f}x faster".format(ratio))
```
As a C++ programmer used to benchmarking with `clock_gettime`, using Python to do benchmarks pains me, but I think the point is clear. If anything I'm handicapped on small matrices due to Python overhead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587066870
The current MXNet quantizer is much slower than intgemm's on a wide variety of matrix sizes.
Experiment on c5.12xlarge:
```
Shape (128, 128)
0.0000864 seconds for quantize
0.0001020 seconds for quantize_v2
0.0000211 seconds for intgemm
0.0001145 seconds for quantize_v2_fit
0.0000409 seconds for intgemm_fit
intgemm is 4.1x faster with calibration
intgemm is 2.8x faster without calibration
Shape (256, 256)
0.0001065 seconds for quantize
0.0001268 seconds for quantize_v2
0.0000259 seconds for intgemm
0.0002040 seconds for quantize_v2_fit
0.0000554 seconds for intgemm_fit
intgemm is 4.1x faster with calibration
intgemm is 3.7x faster without calibration
Shape (512, 512)
0.0001961 seconds for quantize
0.0002111 seconds for quantize_v2
0.0000864 seconds for intgemm
0.0005762 seconds for quantize_v2_fit
0.0001862 seconds for intgemm_fit
intgemm is 2.3x faster with calibration
intgemm is 3.1x faster without calibration
Shape (1024, 1024)
0.0003830 seconds for quantize
0.0003858 seconds for quantize_v2
0.0002519 seconds for intgemm
0.0019079 seconds for quantize_v2_fit
0.0005164 seconds for intgemm_fit
intgemm is 1.5x faster with calibration
intgemm is 3.7x faster without calibration
Shape (2048, 2048)
0.0011643 seconds for quantize
0.0011415 seconds for quantize_v2
0.0008675 seconds for intgemm
0.0072411 seconds for quantize_v2_fit
0.0017965 seconds for intgemm_fit
intgemm is 1.3x faster with calibration
intgemm is 4.0x faster without calibration
Shape (8, 4096)
0.0000920 seconds for quantize
0.0001098 seconds for quantize_v2
0.0000222 seconds for intgemm
0.0001424 seconds for quantize_v2_fit
0.0000452 seconds for intgemm_fit
intgemm is 4.1x faster with calibration
intgemm is 3.1x faster without calibration
Shape (4096, 8)
0.0000921 seconds for quantize
0.0001091 seconds for quantize_v2
0.0000220 seconds for intgemm
0.0001409 seconds for quantize_v2_fit
0.0000456 seconds for intgemm_fit
intgemm is 4.2x faster with calibration
intgemm is 3.1x faster without calibration
```
Generated by `export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=1; taskset --cpu-list 0 ./quant_bench.py`
where `quant_bench.py` is:
```python
#!/usr/bin/env python3
import mxnet as mx
import time
def time_procedure(shape, count, proc):
data = mx.nd.random_uniform(shape=shape, low=-1.0, high = 1.0)
mx.nd.waitall()
begin = time.time()
for i in range(0, count):
proc(data)
mx.nd.waitall()
return (time.time() - begin) / count
shapes = [(128, 128), (256,256), (512, 512), (1024, 1024), (2048, 2048), (8, 4096), (4096, 8)]
count = 1000
one = mx.nd.ones(shape=(1))
minusone = -one
procedures = {
"quantize" : (lambda data : mx.nd.contrib.quantize(data, minusone, one)),
"quantize_v2" : (lambda data : mx.nd.contrib.quantize_v2(data, min_calib_range = -1.0, max_calib_range = 1.0)),
"intgemm" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, one)),
"quantize_v2_fit" : (lambda data : mx.nd.contrib.quantize_v2(data)),
"intgemm_fit" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, mx.nd.contrib.intgemm_maxabsolute(data))),
}
for s in shapes:
print("Shape " + str(s))
stats = {}
for name, l in procedures.items():
stats[name] = time_procedure(s, count, l)
print("{:.7f} seconds for {}".format(stats[name], name))
best_baseline = min(stats["quantize"], stats["quantize_v2"])
ratio = best_baseline / stats["intgemm"]
print("intgemm is {:.1f}x faster with calibration".format(ratio))
fit_ratio = stats["quantize_v2_fit"] / stats["intgemm_fit"]
print("intgemm is {:.1f}x faster without calibration".format(fit_ratio))
```
As a C++ programmer used to benchmarking with `clock_gettime`, using Python to do benchmarks pains me, but I think the point is clear. If anything the Python overhead is pushing results towards 1 by adding to numerator and denominator.
(If you're wondering what changed from previous results, I upgraded to gcc 8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587066870
The current MXNet quantizer is 3-10x slower than intgemm's on a wide variety of matrix sizes.
Experiment on c5.12xlarge:
```
Shape (128, 128)
0.0000731 seconds for quantize
0.0000706 seconds for quantize_v2
0.0000219 seconds for intgemm
intgemm is 3.2x faster
Shape (256, 256)
0.0002116 seconds for quantize
0.0001778 seconds for quantize_v2
0.0000258 seconds for intgemm
intgemm is 6.9x faster
Shape (512, 512)
0.0008112 seconds for quantize
0.0006480 seconds for quantize_v2
0.0000917 seconds for intgemm
intgemm is 7.1x faster
Shape (1024, 1024)
0.0030176 seconds for quantize
0.0023387 seconds for quantize_v2
0.0002542 seconds for intgemm
intgemm is 9.2x faster
Shape (2048, 2048)
0.0118271 seconds for quantize
0.0090704 seconds for quantize_v2
0.0008705 seconds for intgemm
intgemm is 10.4x faster
Shape (8, 4096)
0.0001187 seconds for quantize
0.0001061 seconds for quantize_v2
0.0000226 seconds for intgemm
intgemm is 4.7x faster
```
Generated by `export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=1; taskset --cpu-list 0 ./quant_bench.py`
where `quant_bench.py` is:
```python
#!/usr/bin/env python3
import mxnet as mx
import time
def time_procedure(shape, count, proc):
data = mx.nd.random_uniform(shape=s, low=-1.0, high = 1.0)
mx.nd.waitall()
begin = time.time()
for i in range(0, count):
proc(data)
mx.nd.waitall()
return (time.time() - begin) / count
shapes = [(128, 128), (256,256), (512, 512), (1024, 1024), (2048, 2048)]
count = 1000
one = mx.nd.ones(shape=(1))
minusone = -one
procedures = {
"quantize" : (lambda data : mx.nd.contrib.quantize(data, minusone, one)),
"quantize_v2" : (lambda data : mx.nd.contrib.quantize_v2(data, min_calib_range = -1.0, max_calib_range = 1.0)),
"intgemm" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, one))
}
for s in shapes:
print("Shape " + str(s))
stats = {}
for name, l in procedures.items():
stats[name] = time_procedure(s, count, l)
print("{:.7f} seconds for {}".format(stats[name], name))
best_baseline = min(stats["quantize"], stats["quantize_v2"])
ratio = best_baseline / stats["intgemm"]
print("intgemm is {:.1f}x faster".format(ratio))
```
As a C++ programmer used to benchmarking with `clock_gettime`, using Python to do benchmarks pains me, but I think the point is clear. If anything the Python overhead is pushing results towards 1 by adding to numerator and denominator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587020032
I'm the same person as @kpu but work part time as @kpuatamazon. Typically you'll hear from my Amazon hat on Mondays, though I plan to work flexibly to respond more quickly.
Overall, I think this is going to come down to an end-to-end benchmark.
Here's some numbers from a c5.12xlarge (with VNNI).
Sockeye in fp32 on one core:
```
real 14m21.688s
user 14m24.608s
sys 0m1.329s
```
Sockeye in int8 (intgemm) on one core:
```
real 5m2.986s
user 5m6.203s
sys 0m1.036s
```
And BLEU was unchanged (it went up 0.1% oddly).
I'll work on how much time is spent in GEMM and a version backed with DNNL.
> Also, the intgemm library seems to be a personal project more than a product. I'm not sure how will it be maintained and what's the adoption status in other projects.
> What's the adoption status of the library? And who will maintain the library? Amazon or @kpuatamazon himself?
The intgemm library started as code inside the Marian machine translation project https://marian-nmt.github.io/ . It's been extracted as a standalone. Marian is run in production at Microsoft, the European Union, World Intellectual Property Organization, US Air Force, and others listed on the site. I've introduced @pengzhao-intel to our collaborators at Intel, which has funded some of the development.
I coordinate a 3-year EUR 3 million project funded by the EU to add client-side machine translation to web browsers https://browser.mt/ https://www.zdnet.com/article/firefox-to-get-page-translation-feature-like-chrome/ . This project is using Marian. Since we want to run on people's desktops, intgemm is mostly optimized for pre-VNNI CPUs though we have VNNI support and further register optimization in a branch.
> Could you please be more specific what the functionality is?
> If possible, please share more about how you did the quantization in your gluon model.
I'm calling the quantization operators directly from gluon instead of doing a graph transformation. Please see the Sockeye code that uses this pull request. The code is in https://github.com/awslabs/sockeye/pull/771 and https://github.com/kpuatamazon/sockeye/tree/heafield-quantize
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on issue #17559: [MXNET-1446]
Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
TaoLv commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-585579742
Thank you for the ping, @szha. Generally speaking at a high level, I don't think mxnet needs another int gemm library as we already have MKL BLAS and DNNL support that. One is a BLAS backend of mxnet, and the other is already in the 3rdparty folder. Adding a new dependency will unnecessarily increase the complexity of code as well as the burden of maintenance. Also, the intgemm library seems to be a personal project more than a product. I'm not sure how will it be maintained and what's the adoption status in other projects. Will look into code details later.
Also cc @pengzhao-intel
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590256391
@pengzhao-intel Which OMP do you recommend? I've been getting bad OMP results with a stock install of Ubuntu 18, but am happy to sprinkle a `#pragma omp parallel for` on this embarrassingly parallel task and add a loop at the end to deal with non-multiples of register size.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
pengzhao-intel commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590146883
@kpuatamazon We have tested the performance in our local machine pre- and VNNI feature for both data conversion and computation part (FC layer).
Thanks, @ElaineBao's work on data measurements.
In general, the intgemm can provide better performance on data conversion with less core (1 cores) and this may help on client CPU. However, the computation parts and data conversion with more cores performance are still slower than the current integration. Meanwhile, the supported shape is only limited in number dividable by 4.
**detail data as below**
- FC layer runtime from current MXNet OP with intgemm by 28 cores in 1 socket.
Note: the smaller the better in below table
Thus, the current MXNet OP is still faster in most cases for two-generation CPU.
|CLX8180 VNNI (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.08 | 0.09 | 0.10 | 0.11 | 0.12
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.45 | 0.86 | 1.71
|SKX8280 (non VNNI) (ms/call) | BS=1 | BS=8 | BS=16 | BS=32 | BS=64 | BS=128 |
-- | -- | -- | -- | -- | -- | --
_sg_mkldnn_fully_connected | 0.03 | 0.07 | 0.09 | 0.11 | 0.13 | 0.15
_contrib_intgemm_fully_connected | 0.04 | 0.13 | 0.24 | 0.46 | 0.90 | 1.78
- Data conversion from FP32 to INT8
1 core results show the intgemm has the benefit on data conversion.
shape | quantize_v2 | intgemm | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | -- | -- | --
(128, 128) | 0.062 | 0.0224 | 0.0675 | 0.0442 | 1.5x
(256, 256) | 0.0794 | 0.026 | 0.1264 | 0.0577 | 2.2x
(512, 512) | 0.1615 | 0.0863 | 0.4025 | 0.1757 | 2.3x
(1024, 1024) | 0.3422 | 0.2485 | 1.3097 | 0.5153 | 2.5x
(2048, 2048) | 1.0238 | 0.8415 | 4.7436 | 1.7523 | 2.7x
(8, 4096) | 0.0666 | 0.0235 | 0.0845 | 0.0485 | 1.7x
(4096, 8) | 0.0657 | 0.0236 | 0.0841 | 0.0482 | 1.7x
But with more cores (28 cores), the current OP is slight better.
shape | quantize_v2 | intgemm | quantize_v2_fit | intgemm_fit | Speedup without calib
-- | -- | -- | -- | -- | --
(128, 128) | 0.0757 | 0.0356 | 0.0767 | 0.0704 | 1.1x
(256, 256) | 0.0773 | 0.0408 | 0.084 | 0.0907 | 0.9x
(512, 512) | 0.0798 | 0.0949 | 0.1083 | 0.2163 | 0.5x
(1024, 1024) | 0.0892 | 0.244 | 0.1645 | 0.5415 | 0.3x
(2048, 2048) | 0.1441 | 0.76 | 0.3507 | 1.769 | 0.2x
(8, 4096) | 0.0752 | 0.037 | 0.0798 | 0.0766 | 1.0x
(4096, 8) | 0.0755 | 0.0372 | 0.0795 | 0.0771 | 1.0x
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-586641284
> You may consider the custom operator / subgraph feature developed by @samskalicky 's team.
Thanks @TaoLv. Hi @mjdenkowski and @kpuatamazon, here are some pointers to what @TaoLv mentioned, it may or may not be a good fit for what you're doing. If you have more questions about this please reply to the appropriate PRs:
- Documentation for custom operators is WIP here: https://github.com/apache/incubator-mxnet/blob/448ea22a6aa3545c2371d18efa9242354dec64de/example/extensions/lib_custom_op/README.md
- Documentation for custom subgraphing is WIP here: https://github.com/apache/incubator-mxnet/blob/08bddfcff928a85d4573ea7c5182c76543f23c5c/example/extensions/lib_subgraph/README.md
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on issue #17559: [MXNET-1446]
Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
TaoLv commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-586591074
Hi @mjdenkowski ,
> We're particularly interested in Kenneth's (@kpuatamazon) intgemm because it provides functionality we weren't able to find in the existing libraries.
Could you please be more specific what the functionality is?
> we're seeing a roughly 3X inference speedup on an already significantly optimized transformer implementation.
It sounds to be a great achievement. Could you please share more detail numbers of that? Ie. how much time is spent in fp32/int8 gemm before and after the optimization?
> Like many other Gluon users, our inference model is not currently expressible as a static graph.
I know there is a plan/effort of moving the existing quantization flow from the symbolic executor to gluon. It's part of the roadmap of mxnet 2.0. If possible, please share more about how you did the quantization in your gluon model. It will help mxnet to improve. Thanks!
> Are there particular concerns we can address about adding intgemm as a third party library?
Sorry i haven't read the code yet. But I would like to know what's the advantage of this library over the existing MKL BLAS and DNNL? What's the adoption status of the library? And who will maintain the library? Amazon or @kpuatamazon himself?
> Is there another path to using intgemm with MXNet that you recommend?
You may consider the custom operator / subgraph feature developed by @samskalicky 's team.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mjdenkowski commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mjdenkowski commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-586425627
Hi @TaoLv, thanks for taking a look at this! We understand your comments about MXNet already having multiple GEMM libraries. We're particularly interested in Kenneth's (@kpuatamazon) intgemm because it provides functionality we weren't able to find in the existing libraries. As he mentioned in the PR, we're seeing a roughly 3X inference speedup on an already significantly optimized transformer implementation. Like many other Gluon users, our inference model is not currently expressible as a static graph.
We would like to work out the best way to make this functionality available to the larger community. Are there particular concerns we can address about adding intgemm as a third party library? Is there another path to using intgemm with MXNet that you recommend?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587066870
The current MXNet quantizer is 3-10x slower than intgemm's on a wide variety of matrix sizes.
Experiment on c5.12xlarge:
```
Shape (128, 128)
0.0000731 seconds for quantize
0.0000706 seconds for quantize_v2
0.0000219 seconds for intgemm
intgemm is 3.2x faster
Shape (256, 256)
0.0002116 seconds for quantize
0.0001778 seconds for quantize_v2
0.0000258 seconds for intgemm
intgemm is 6.9x faster
Shape (512, 512)
0.0008112 seconds for quantize
0.0006480 seconds for quantize_v2
0.0000917 seconds for intgemm
intgemm is 7.1x faster
Shape (1024, 1024)
0.0030176 seconds for quantize
0.0023387 seconds for quantize_v2
0.0002542 seconds for intgemm
intgemm is 9.2x faster
Shape (2048, 2048)
0.0118271 seconds for quantize
0.0090704 seconds for quantize_v2
0.0008705 seconds for intgemm
intgemm is 10.4x faster
Shape (8, 4096)
0.0001187 seconds for quantize
0.0001061 seconds for quantize_v2
0.0000226 seconds for intgemm
intgemm is 4.7x faster
```
Generated by `export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=1; taskset --cpu-list 0 ./quant_bench.py`
where `quant_bench.py` is:
```python
#!/usr/bin/env python3
import mxnet as mx
import time
def time_procedure(shape, count, proc):
data = mx.nd.random_uniform(shape=shape, low=-1.0, high = 1.0)
mx.nd.waitall()
begin = time.time()
for i in range(0, count):
proc(data)
mx.nd.waitall()
return (time.time() - begin) / count
shapes = [(128, 128), (256,256), (512, 512), (1024, 1024), (2048, 2048)]
count = 1000
one = mx.nd.ones(shape=(1))
minusone = -one
procedures = {
"quantize" : (lambda data : mx.nd.contrib.quantize(data, minusone, one)),
"quantize_v2" : (lambda data : mx.nd.contrib.quantize_v2(data, min_calib_range = -1.0, max_calib_range = 1.0)),
"intgemm" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, one))
}
for s in shapes:
print("Shape " + str(s))
stats = {}
for name, l in procedures.items():
stats[name] = time_procedure(s, count, l)
print("{:.7f} seconds for {}".format(stats[name], name))
best_baseline = min(stats["quantize"], stats["quantize_v2"])
ratio = best_baseline / stats["intgemm"]
print("intgemm is {:.1f}x faster".format(ratio))
```
As a C++ programmer used to benchmarking with `clock_gettime`, using Python to do benchmarks pains me, but I think the point is clear. If anything the Python overhead is pushing results towards 1 by adding to numerator and denominator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-593952925
The quantization operator is now parallelized with OpenMP and supports an arbitrary number of arguments. It is substantially faster than the current MXNet implementation on both 1 and 24 cores (see below for benchmarks) @pengzhao-intel .
Maybe this is too big of a pull request. Would you be happy with a smaller pull request that takes the faster quantization code and replaces the implementation of the existing quantize and quantize_v2 operators, so it also appears in the quantization flow?
Then we can carry on with matrix multiply next.
@ciyongch I'm calling operators manually because we're using gluon and the quantization workflow doesn't work for us anyway. But if you're game to have operators optimized, they'll automatically be in the workflow too.
```
OMP_NUM_THREADS=24 ./quant_bench.py
Shape (1, 1)
0.0001304 seconds for quantize
0.0001076 seconds for quantize_v2
0.0000310 seconds for intgemm
0.0001114 seconds for quantize_v2_fit
0.0000479 seconds for intgemm_fit
intgemm is 3.5x faster with calibration
intgemm is 2.3x faster without calibration
Shape (128, 128)
0.0001649 seconds for quantize
0.0001399 seconds for quantize_v2
0.0000329 seconds for intgemm
0.0001533 seconds for quantize_v2_fit
0.0000502 seconds for intgemm_fit
intgemm is 4.2x faster with calibration
intgemm is 3.1x faster without calibration
Shape (256, 256)
0.0001660 seconds for quantize
0.0001404 seconds for quantize_v2
0.0000335 seconds for intgemm
0.0001599 seconds for quantize_v2_fit
0.0000505 seconds for intgemm_fit
intgemm is 4.2x faster with calibration
intgemm is 3.2x faster without calibration
Shape (512, 512)
0.0001691 seconds for quantize
0.0001434 seconds for quantize_v2
0.0000342 seconds for intgemm
0.0001813 seconds for quantize_v2_fit
0.0000540 seconds for intgemm_fit
intgemm is 4.2x faster with calibration
intgemm is 3.4x faster without calibration
Shape (1024, 1024)
0.0001920 seconds for quantize
0.0001538 seconds for quantize_v2
0.0000511 seconds for intgemm
0.0002390 seconds for quantize_v2_fit
0.0000827 seconds for intgemm_fit
intgemm is 3.0x faster with calibration
intgemm is 2.9x faster without calibration
Shape (2048, 2048)
0.0002364 seconds for quantize
0.0001989 seconds for quantize_v2
0.0000875 seconds for intgemm
0.0004747 seconds for quantize_v2_fit
0.0001531 seconds for intgemm_fit
intgemm is 2.3x faster with calibration
intgemm is 3.1x faster without calibration
Shape (20971520,)
0.0011446 seconds for quantize
0.0010902 seconds for quantize_v2
0.0008950 seconds for intgemm
0.0023337 seconds for quantize_v2_fit
0.0015005 seconds for intgemm_fit
intgemm is 1.2x faster with calibration
intgemm is 1.6x faster without calibration
Shape (8, 4096)
0.0001636 seconds for quantize
0.0001392 seconds for quantize_v2
0.0000364 seconds for intgemm
0.0001508 seconds for quantize_v2_fit
0.0000651 seconds for intgemm_fit
intgemm is 3.8x faster with calibration
intgemm is 2.3x faster without calibration
Shape (4096, 8)
0.0001642 seconds for quantize
0.0001392 seconds for quantize_v2
0.0000370 seconds for intgemm
0.0001515 seconds for quantize_v2_fit
0.0000654 seconds for intgemm_fit
intgemm is 3.8x faster with calibration
intgemm is 2.3x faster without calibration
```
```
OMP_NUM_THREADS=1 ./quant_bench.py
Shape (1, 1)
0.0000630 seconds for quantize
0.0000706 seconds for quantize_v2
0.0000294 seconds for intgemm
0.0000632 seconds for quantize_v2_fit
0.0000475 seconds for intgemm_fit
intgemm is 2.1x faster with calibration
intgemm is 1.3x faster without calibration
Shape (128, 128)
0.0000860 seconds for quantize
0.0000898 seconds for quantize_v2
0.0000324 seconds for intgemm
0.0000996 seconds for quantize_v2_fit
0.0000464 seconds for intgemm_fit
intgemm is 2.6x faster with calibration
intgemm is 2.1x faster without calibration
Shape (256, 256)
0.0000976 seconds for quantize
0.0001028 seconds for quantize_v2
0.0000339 seconds for intgemm
0.0001513 seconds for quantize_v2_fit
0.0000521 seconds for intgemm_fit
intgemm is 2.9x faster with calibration
intgemm is 2.9x faster without calibration
Shape (512, 512)
0.0001724 seconds for quantize
0.0001693 seconds for quantize_v2
0.0000839 seconds for intgemm
0.0004351 seconds for quantize_v2_fit
0.0001420 seconds for intgemm_fit
intgemm is 2.0x faster with calibration
intgemm is 3.1x faster without calibration
Shape (1024, 1024)
0.0003559 seconds for quantize
0.0003481 seconds for quantize_v2
0.0002384 seconds for intgemm
0.0013803 seconds for quantize_v2_fit
0.0004667 seconds for intgemm_fit
intgemm is 1.5x faster with calibration
intgemm is 3.0x faster without calibration
Shape (2048, 2048)
0.0011425 seconds for quantize
0.0010880 seconds for quantize_v2
0.0008497 seconds for intgemm
0.0051828 seconds for quantize_v2_fit
0.0018427 seconds for intgemm_fit
intgemm is 1.3x faster with calibration
intgemm is 2.8x faster without calibration
Shape (20971520,)
0.0101917 seconds for quantize
0.0096956 seconds for quantize_v2
0.0071391 seconds for intgemm
0.0305159 seconds for quantize_v2_fit
0.0140535 seconds for intgemm_fit
intgemm is 1.4x faster with calibration
intgemm is 2.2x faster without calibration
Shape (8, 4096)
0.0000880 seconds for quantize
0.0000950 seconds for quantize_v2
0.0000334 seconds for intgemm
0.0001183 seconds for quantize_v2_fit
0.0000423 seconds for intgemm_fit
intgemm is 2.6x faster with calibration
intgemm is 2.8x faster without calibration
Shape (4096, 8)
0.0000900 seconds for quantize
0.0000949 seconds for quantize_v2
0.0000332 seconds for intgemm
0.0001215 seconds for quantize_v2_fit
0.0000433 seconds for intgemm_fit
intgemm is 2.7x faster with calibration
intgemm is 2.8x faster without calibration
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-590363395
While I agree with the principle that all operators should be parallel (and intend to parallelize mine), it's important to find the optimal place for parallelism.
This benchmark shows the right place is over sentences, not inside operators, at least with Sockeye on current MXNet. In Marian, we've turned off all OMP threading and just use sentence-level parallelism.
Setup: float32 models using MKL and 1 sentence at a time (admittedly small, but common use case in inference). Nothing special from this pull request.
One OMP thread, parallelize across sentences with separate processes.
```bash
export OMP_NUM_THREADS=1
time parallel --block 10k --line-buffer --pipe -k python3 -m sockeye.translate --use-cpu -m model --restrict-lexicon model/lexicon --beam-size 5 <test
```
```
real 1m17.884s
user 37m32.076s
sys 1m38.002s
```
This version loaded Sockeye 41 times! Each of these takes:
```
real 0m3.528s
user 0m7.050s
sys 0m1.240s
```
Use all 24 hyperthreads for OMP:
```bash
unset OMP_NUM_THREADS
python3 -m sockeye.translate --use-cpu -m model --restrict-lexicon model/lexicon --beam-size 5 <test
```
```
real 4m26.204s
user 103m50.951s
sys 2m10.632s
```
This is in line with results reported with Intel, though I just took it further to one process per core.
https://raw.githubusercontent.com/awslabs/sockeye/master/docs/tutorials/cpu_process_per_core_translation.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel commented on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
pengzhao-intel commented on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-586665115
> Hi @TaoLv, thanks for taking a look at this! We understand your comments about MXNet already having multiple GEMM libraries. We're particularly interested in Kenneth's (@kpuatamazon) intgemm because it provides functionality we weren't able to find in the existing libraries. As he mentioned in the PR, we're seeing a roughly 3X inference speedup on an already significantly optimized transformer implementation. Like many other Gluon users, our inference model is not currently expressible as a static graph.
>
> We would like to work out the best way to make this functionality available to the larger community. Are there particular concerns we can address about adding intgemm as a third party library? Is there another path to using intgemm with MXNet that you recommend?
We have the experience to quantize the sockeye models previously and also talked with @fhieber (cc @ciyongch) and we saw the solid speedup as well. Thus, we're highly interested in your models to figure out the functionality difference as you mentioned.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on issue #17559:
[MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on issue #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-587066870
The current MXNet quantizer is 3-10x slower than intgemm's on a wide variety of matrix sizes.
Experiment on c5.12xlarge:
```
Shape (128, 128)
0.0000731 seconds for quantize
0.0000706 seconds for quantize_v2
0.0000219 seconds for intgemm
intgemm is 3.2x faster
Shape (256, 256)
0.0002116 seconds for quantize
0.0001778 seconds for quantize_v2
0.0000258 seconds for intgemm
intgemm is 6.9x faster
Shape (512, 512)
0.0008112 seconds for quantize
0.0006480 seconds for quantize_v2
0.0000917 seconds for intgemm
intgemm is 7.1x faster
Shape (1024, 1024)
0.0030176 seconds for quantize
0.0023387 seconds for quantize_v2
0.0002542 seconds for intgemm
intgemm is 9.2x faster
Shape (2048, 2048)
0.0118271 seconds for quantize
0.0090704 seconds for quantize_v2
0.0008705 seconds for intgemm
intgemm is 10.4x faster
Shape (8, 4096)
0.0001187 seconds for quantize
0.0001061 seconds for quantize_v2
0.0000226 seconds for intgemm
intgemm is 4.7x faster
```
Generated by `export MXNET_ENGINE_TYPE=NaiveEngine; export OMP_NUM_THREADS=1; taskset --cpu-list 0 ./quant_bench.py`
where `quant_bench.py` is:
```
#!/usr/bin/env python3
import mxnet as mx
import time
def time_procedure(shape, count, proc):
data = mx.nd.random_uniform(shape=s, low=-1.0, high = 1.0)
mx.nd.waitall()
begin = time.time()
for i in range(0, count):
proc(data)
mx.nd.waitall()
return (time.time() - begin) / count
shapes = [(128, 128), (256,256), (512, 512), (1024, 1024), (2048, 2048)]
count = 1000
one = mx.nd.ones(shape=(1))
minusone = -one
procedures = {
"quantize" : (lambda data : mx.nd.contrib.quantize(data, minusone, one)),
"quantize_v2" : (lambda data : mx.nd.contrib.quantize_v2(data, min_calib_range = -1.0, max_calib_range = 1.0)),
"intgemm" : (lambda data : mx.nd.contrib.intgemm_prepare_data(data, one))
}
for s in shapes:
print("Shape " + str(s))
stats = {}
for name, l in procedures.items():
stats[name] = time_procedure(s, count, l)
print("{:.7f} seconds for {}".format(stats[name], name))
best_baseline = min(stats["quantize"], stats["quantize_v2"])
ratio = best_baseline / stats["intgemm"]
print("intgemm is {:.1f}x faster".format(ratio))
```
As a C++ programmer used to benchmarking with `clock_gettime`, using Python to do benchmarks pains me, but I think the point is clear. If anything the Python overhead is pushing results towards 1 by adding to numerator and denominator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479475733
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
I've removed `include_directories(${intgemm_BINARY_DIR})`.
However, `include_directories(${intgemm_SOURCE_DIR})` remains. intgemm itself has entirely relative header paths and users do not need to `-I` the source directory to use it, so it would be incorrect to add that to the library.
The only reason is that you asked for `FetchContent` which implies the code moves around which implies that MXNet needs some what to find the header files to begin with. Were this to convert back to a submodule, the path to intgemm would be constant and `include_directories(${intgemm_SOURCE_DIR})` could be removed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646332
Undefined action detected.
Permissible actions are : run ci [all], run ci [job1, job2]
Example : @mxnet-bot run ci [all]
Example : @mxnet-bot run ci [centos-cpu, clang]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646058
@mxnet-bot run [centos-gpu, unix-cpu, unix-gpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479473751
##########
File path: CMakeLists.txt
##########
@@ -64,6 +64,11 @@ if(USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND (CMAKE_HOST_SYSTEM_PR
else()
option(USE_MKLDNN "Build with MKL-DNN support" OFF)
endif()
+if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT CMAKE_CROSSCOMPILING))
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" ON)
+else()
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" OFF)
+endif()
Review comment:
8aaa23c5b1f1b3dbef78be229abdc83dbe96216d
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r460228415
##########
File path: src/operator/contrib/intgemm/prepare_weight_op.cc
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file prepare_weight_op.cc
+ * \brief Converts weight matrices to intgemm's representation.
+ */
+
+#include <mxnet/operator_util.h>
+#include <vector>
+#include "../../mshadow_op.h"
+#include "../../mxnet_op.h"
+#include "../../operator_common.h"
+#include "../../tensor/init_op.h"
+
+#include "../../../../3rdparty/intgemm/aligned.h"
+#include "../../../../3rdparty/intgemm/intgemm.h"
+
+namespace mxnet {
+namespace op {
+
+struct PrepareWeightParam : public dmlc::Parameter<PrepareWeightParam> {
+ bool already_quantized;
+ DMLC_DECLARE_PARAMETER(PrepareWeightParam) {
+ DMLC_DECLARE_FIELD(already_quantized).set_default(false)
+ .describe("Is the weight matrix already quantized?");
+ }
+};
+DMLC_REGISTER_PARAMETER(PrepareWeightParam);
+
+bool PrepareWeightOpShape(const nnvm::NodeAttrs& attrs,
+ mxnet::ShapeVector* in_attrs,
+ mxnet::ShapeVector* out_attrs) {
+ // Optimal maximum parameter.
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+
+ SHAPE_ASSIGN_CHECK(*out_attrs, 0, in_attrs->at(0));
+ SHAPE_ASSIGN_CHECK(*in_attrs, 0, out_attrs->at(0));
+
+ if (in_attrs->size() == 2U) {
+ SHAPE_ASSIGN_CHECK(*in_attrs, 1, mxnet::TShape(1, 1));
+ }
+ return shape_is_known(out_attrs->at(0));
+}
+
+bool PrepareWeightOpType(const nnvm::NodeAttrs& attrs,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ TYPE_ASSIGN_CHECK(*out_attrs, 0, mshadow::kInt8);
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ if (in_attrs->size() == 1U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kInt8);
+ } else if (in_attrs->size() == 2U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kFloat32);
+ TYPE_ASSIGN_CHECK(*in_attrs, 1, mshadow::kFloat32);
+ }
+ return true;
+}
+
+bool PrepareWeightOpStorageType(const nnvm::NodeAttrs& attrs,
+ const int dev_mask,
+ DispatchMode* dispatch_mode,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+ STORAGE_TYPE_ASSIGN_CHECK(*out_attrs, 0, kDefaultStorage);
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 0, kDefaultStorage);
+ if (in_attrs->size() == 2U) {
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 1, kDefaultStorage);
+ }
+ DISPATCH_MODE_ASSIGN_CHECK(dispatch_mode, 0, DispatchMode::kFComputeEx);
+ return true;
+}
+
+void PrepareWeightOpForwardCPU(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector<TBlob>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<TBlob>& outputs) {
+ const PrepareWeightParam& params = nnvm::get<PrepareWeightParam>(attrs.parsed);
+ CHECK_EQ(inputs.size(), params.already_quantized ? 1U : 2U);
+ CHECK_EQ(outputs.size(), 1U);
+ CHECK_EQ(req.size(), 1U);
+ CHECK_EQ(req[0], kWriteTo) << "intgemm only overwrites";
+
+ const TBlob &in = inputs.front();
+ const TBlob &out = outputs.front();
+ CHECK_EQ(out.type_flag_, mshadow::kInt8);
+ CHECK(in.CheckContiguous());
+ CHECK(out.CheckContiguous());
+ size_t B_cols = in.shape_.ProdShape(0, in.shape_.ndim() - 1);
+ size_t inner = in.shape_[in.shape_.ndim() - 1];
+ CHECK_EQ(inner % ::intgemm::Int8::tile_info.b_rows, 0) <<
+ "intgemm requires the inner dimension be a multiple of " << ::intgemm::Int8::tile_info.b_rows;
+ CHECK_EQ(B_cols % ::intgemm::Int8::tile_info.b_cols, 0) <<
+ "intgemm requires the output dimension (the product of all but the last dimension of the "
+ "weight matrix) to be a multiple of " << ::intgemm::Int8::tile_info.b_cols << ".";
+
+ int8_t *quantB = out.dptr<int8_t>();
+ CHECK(in.type_flag_ == mshadow::kFloat32 || in.type_flag_ == mshadow::kInt8) <<
+ "Expected either 32-bit values to be quantized or 8-bit values to rearrange.";
+ if (in.type_flag_ == mshadow::kInt8) {
+ const int8_t *B = in.dptr<int8_t>();
+ ::intgemm::Int8::PrepareBQuantizedTransposed(B, quantB, inner, B_cols);
+ } else if (in.type_flag_ == mshadow::kFloat32) {
+ const float *B = in.dptr<float>();
+ // TODO(kpuatamazon): eliminate transpose here with https://github.com/kpu/intgemm/pull/56
+ intgemm::AlignedVector<float> B_transpose(inner * B_cols);
+ for (size_t i = 0; i < inner; ++i) {
+ for (size_t j = 0; j < B_cols; ++j) {
+ B_transpose[i * B_cols + j] = B[i + inner * j];
+ }
+ }
+ ::intgemm::Int8::PrepareB(
+ B_transpose.begin(),
+ quantB,
+ 127.0 / *inputs[1].dptr<float>(),
+ inner,
+ B_cols);
+ }
+}
+
+NNVM_REGISTER_OP(_contrib_intgemm_prepare_weight)
+.describe(R"code(This operator converts a weight matrix in column-major format to intgemm's internal fast representation of weight matrices. MXNet customarily stores weight matrices in column-major (transposed) format. This operator is not meant to be fast; it is meant to be run offline to quantize a model.
Review comment:
I'm interested in your suggestions on how best to do this. Keep in mind we do have a use case for storing 8-bit quantized models on disk.
Also, oneDNN has hidden support for an 8-bit packed representation as well that isn't used in MXNet to my knowledge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646058
@mxnet-bot run ci [centos-gpu, unix-cpu, unix-gpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479412917
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
33ad78208c911a5e9d770416266b0d3510767213 now uses `target_link_libraries(mxnet PRIVATE intgemm)`
Regarding `INTGEMM_OPERATOR_SOURCE`, that's not adding stuff, it's removing the MXNet-side operator implementations (which were caught by the glob) if the user elects to not compile support.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon edited a comment on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon edited a comment on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646058
@mxnet-bot run ci
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479410149
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
Sorry for being unclear in the previous comment. What I mean is, that if you use `add_subdirectory`, there is no need for adding `INTGEMM_OPERATOR_SOURCE` manually to the list of source files of the mxnet library. Instead the `CMakeLists.txt` should declare a target, which encapsulates the set of source and header files (and triggers the compilation tests). Then, in the mxnet CMakeLists.txt you only need `add_subdirectory` and `target_link_libraries(mxnet PRIVATE intgemm)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-682958853
@leezu Ready?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479496991
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> If you want to use FetchContent then the directory is variable and there needs to be some mechanism for the MXNet code to find intgemm.
Essentially you need to update the intgemm cmakelists file to ensure it encapsulates the headers as part of the intgemm target. Once you do that, cmake will handle finding intgemm header files correctly. The reason that things break once you use `FetchContent` is that the `intgemm/CMakeLists.txt` is not correct.
Currently you're hardcoding the ` #include "../../../../3rdparty/intgemm/intgemm.h"` path inside the MXNet source files. It works, but it doesn't follow the best practice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r474193172
##########
File path: tests/python/unittest/test_contrib_intgemm.py
##########
@@ -0,0 +1,205 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
for master branch, we intend to move most of the specialized ops to the `npx` space, and it would be great if you could register an alias there too and add a numpy version of the following tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479501831
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> Currently you're hardcoding the `#include "../../../../3rdparty/intgemm/intgemm.h"` path inside the MXNet source files.
This is common practice:
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/common/tensor_inspector.h#L35
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cc#L25
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cu#L27
> Essentially you need to update the intgemm cmakelists file to ensure it encapsulates the headers as part of the intgemm target.
Were I to do that, it would unnecessarily pollute the include paths of software that references the file by its path in the source/submodule tree.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479357884
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
Added, though I'm a bit worried about polluting the includes since there's no longer a 3rdparty/intgemm prefix to them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r460226662
##########
File path: src/operator/contrib/intgemm/prepare_weight_op.cc
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file prepare_weight_op.cc
+ * \brief Converts weight matrices to intgemm's representation.
+ */
+
+#include <mxnet/operator_util.h>
+#include <vector>
+#include "../../mshadow_op.h"
+#include "../../mxnet_op.h"
+#include "../../operator_common.h"
+#include "../../tensor/init_op.h"
+
+#include "../../../../3rdparty/intgemm/aligned.h"
+#include "../../../../3rdparty/intgemm/intgemm.h"
+
+namespace mxnet {
+namespace op {
+
+struct PrepareWeightParam : public dmlc::Parameter<PrepareWeightParam> {
+ bool already_quantized;
+ DMLC_DECLARE_PARAMETER(PrepareWeightParam) {
+ DMLC_DECLARE_FIELD(already_quantized).set_default(false)
+ .describe("Is the weight matrix already quantized?");
+ }
+};
+DMLC_REGISTER_PARAMETER(PrepareWeightParam);
+
+bool PrepareWeightOpShape(const nnvm::NodeAttrs& attrs,
+ mxnet::ShapeVector* in_attrs,
+ mxnet::ShapeVector* out_attrs) {
+ // Optimal maximum parameter.
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+
+ SHAPE_ASSIGN_CHECK(*out_attrs, 0, in_attrs->at(0));
+ SHAPE_ASSIGN_CHECK(*in_attrs, 0, out_attrs->at(0));
+
+ if (in_attrs->size() == 2U) {
+ SHAPE_ASSIGN_CHECK(*in_attrs, 1, mxnet::TShape(1, 1));
+ }
+ return shape_is_known(out_attrs->at(0));
+}
+
+bool PrepareWeightOpType(const nnvm::NodeAttrs& attrs,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ TYPE_ASSIGN_CHECK(*out_attrs, 0, mshadow::kInt8);
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ if (in_attrs->size() == 1U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kInt8);
+ } else if (in_attrs->size() == 2U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kFloat32);
+ TYPE_ASSIGN_CHECK(*in_attrs, 1, mshadow::kFloat32);
+ }
+ return true;
+}
+
+bool PrepareWeightOpStorageType(const nnvm::NodeAttrs& attrs,
+ const int dev_mask,
+ DispatchMode* dispatch_mode,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+ STORAGE_TYPE_ASSIGN_CHECK(*out_attrs, 0, kDefaultStorage);
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 0, kDefaultStorage);
+ if (in_attrs->size() == 2U) {
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 1, kDefaultStorage);
+ }
+ DISPATCH_MODE_ASSIGN_CHECK(dispatch_mode, 0, DispatchMode::kFComputeEx);
+ return true;
+}
+
+void PrepareWeightOpForwardCPU(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector<TBlob>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<TBlob>& outputs) {
+ const PrepareWeightParam& params = nnvm::get<PrepareWeightParam>(attrs.parsed);
+ CHECK_EQ(inputs.size(), params.already_quantized ? 1U : 2U);
+ CHECK_EQ(outputs.size(), 1U);
+ CHECK_EQ(req.size(), 1U);
+ CHECK_EQ(req[0], kWriteTo) << "intgemm only overwrites";
+
+ const TBlob &in = inputs.front();
+ const TBlob &out = outputs.front();
+ CHECK_EQ(out.type_flag_, mshadow::kInt8);
+ CHECK(in.CheckContiguous());
+ CHECK(out.CheckContiguous());
+ size_t B_cols = in.shape_.ProdShape(0, in.shape_.ndim() - 1);
+ size_t inner = in.shape_[in.shape_.ndim() - 1];
+ CHECK_EQ(inner % ::intgemm::Int8::tile_info.b_rows, 0) <<
+ "intgemm requires the inner dimension be a multiple of " << ::intgemm::Int8::tile_info.b_rows;
+ CHECK_EQ(B_cols % ::intgemm::Int8::tile_info.b_cols, 0) <<
+ "intgemm requires the output dimension (the product of all but the last dimension of the "
+ "weight matrix) to be a multiple of " << ::intgemm::Int8::tile_info.b_cols << ".";
+
+ int8_t *quantB = out.dptr<int8_t>();
+ CHECK(in.type_flag_ == mshadow::kFloat32 || in.type_flag_ == mshadow::kInt8) <<
+ "Expected either 32-bit values to be quantized or 8-bit values to rearrange.";
+ if (in.type_flag_ == mshadow::kInt8) {
+ const int8_t *B = in.dptr<int8_t>();
+ ::intgemm::Int8::PrepareBQuantizedTransposed(B, quantB, inner, B_cols);
+ } else if (in.type_flag_ == mshadow::kFloat32) {
+ const float *B = in.dptr<float>();
+ // TODO(kpuatamazon): eliminate transpose here with https://github.com/kpu/intgemm/pull/56
+ intgemm::AlignedVector<float> B_transpose(inner * B_cols);
+ for (size_t i = 0; i < inner; ++i) {
+ for (size_t j = 0; j < B_cols; ++j) {
+ B_transpose[i * B_cols + j] = B[i + inner * j];
+ }
+ }
+ ::intgemm::Int8::PrepareB(
+ B_transpose.begin(),
+ quantB,
+ 127.0 / *inputs[1].dptr<float>(),
+ inner,
+ B_cols);
+ }
+}
+
+NNVM_REGISTER_OP(_contrib_intgemm_prepare_weight)
+.describe(R"code(This operator converts a weight matrix in column-major format to intgemm's internal fast representation of weight matrices. MXNet customarily stores weight matrices in column-major (transposed) format. This operator is not meant to be fast; it is meant to be run offline to quantize a model.
Review comment:
this use case sounds similar to the sparse storage or mkldnn storage use cases. I don't think we ever expose the implementation-specific layout in the frontend.
@eric-haibin-lin how would you suggest to handle the weight layout conversion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-671495186
Tests added. Note they currently don't work due to broken 64 byte alignment #18854.
I have merged the 64 byte alignment fix #18885 and intgemm into another branch: https://github.com/kpuatamazon/incubator-mxnet/tree/intgemm_fixallocator .
Just didn't want to have one pull request include another pull request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479484233
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
I've pushed a submodule version without any `include_directories`. If you want to use `FetchContent` then the directory is variable and there needs to be some mechanism for the MXNet code to find `intgemm`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479327945
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
Think this was meant to be a comment on `add_subdirectory`? The minimal build needs `intgemm.cc` compiled in and these compilation tests that generate a header file: https://github.com/kpu/intgemm/blob/0f05c3ebd037eacdf8cff165736fea2b0d125023/CMakeLists.txt#L20-L37
Not sure how to make `cmake` run the compilation tests without `add_subdirectory`? I can do `add_subdirectory( 3rdparty/intgemm EXCLUDE_FROM_ALL)` if that helps.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-640765886
@ChaiBapchya merged and tested with cmake. Apparently OS X doesn't like the +x constraint in inline asm. Will have to find a mac somewhere...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663645682
Mac OS is fixed. The test failures look unrelated to my code.
@mxnet-bot run ci all
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r460228415
##########
File path: src/operator/contrib/intgemm/prepare_weight_op.cc
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file prepare_weight_op.cc
+ * \brief Converts weight matrices to intgemm's representation.
+ */
+
+#include <mxnet/operator_util.h>
+#include <vector>
+#include "../../mshadow_op.h"
+#include "../../mxnet_op.h"
+#include "../../operator_common.h"
+#include "../../tensor/init_op.h"
+
+#include "../../../../3rdparty/intgemm/aligned.h"
+#include "../../../../3rdparty/intgemm/intgemm.h"
+
+namespace mxnet {
+namespace op {
+
+struct PrepareWeightParam : public dmlc::Parameter<PrepareWeightParam> {
+ bool already_quantized;
+ DMLC_DECLARE_PARAMETER(PrepareWeightParam) {
+ DMLC_DECLARE_FIELD(already_quantized).set_default(false)
+ .describe("Is the weight matrix already quantized?");
+ }
+};
+DMLC_REGISTER_PARAMETER(PrepareWeightParam);
+
+bool PrepareWeightOpShape(const nnvm::NodeAttrs& attrs,
+ mxnet::ShapeVector* in_attrs,
+ mxnet::ShapeVector* out_attrs) {
+ // Optimal maximum parameter.
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+
+ SHAPE_ASSIGN_CHECK(*out_attrs, 0, in_attrs->at(0));
+ SHAPE_ASSIGN_CHECK(*in_attrs, 0, out_attrs->at(0));
+
+ if (in_attrs->size() == 2U) {
+ SHAPE_ASSIGN_CHECK(*in_attrs, 1, mxnet::TShape(1, 1));
+ }
+ return shape_is_known(out_attrs->at(0));
+}
+
+bool PrepareWeightOpType(const nnvm::NodeAttrs& attrs,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ TYPE_ASSIGN_CHECK(*out_attrs, 0, mshadow::kInt8);
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ if (in_attrs->size() == 1U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kInt8);
+ } else if (in_attrs->size() == 2U) {
+ TYPE_ASSIGN_CHECK(*in_attrs, 0, mshadow::kFloat32);
+ TYPE_ASSIGN_CHECK(*in_attrs, 1, mshadow::kFloat32);
+ }
+ return true;
+}
+
+bool PrepareWeightOpStorageType(const nnvm::NodeAttrs& attrs,
+ const int dev_mask,
+ DispatchMode* dispatch_mode,
+ std::vector<int>* in_attrs,
+ std::vector<int>* out_attrs) {
+ CHECK_GE(in_attrs->size(), 1U) << "Need at least weight to quantize.";
+ CHECK_LE(in_attrs->size(), 2U) << "weight and maximum for scaling.";
+ CHECK_EQ(out_attrs->size(), 1U);
+ STORAGE_TYPE_ASSIGN_CHECK(*out_attrs, 0, kDefaultStorage);
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 0, kDefaultStorage);
+ if (in_attrs->size() == 2U) {
+ STORAGE_TYPE_ASSIGN_CHECK(*in_attrs, 1, kDefaultStorage);
+ }
+ DISPATCH_MODE_ASSIGN_CHECK(dispatch_mode, 0, DispatchMode::kFComputeEx);
+ return true;
+}
+
+void PrepareWeightOpForwardCPU(const nnvm::NodeAttrs& attrs,
+ const OpContext& ctx,
+ const std::vector<TBlob>& inputs,
+ const std::vector<OpReqType>& req,
+ const std::vector<TBlob>& outputs) {
+ const PrepareWeightParam& params = nnvm::get<PrepareWeightParam>(attrs.parsed);
+ CHECK_EQ(inputs.size(), params.already_quantized ? 1U : 2U);
+ CHECK_EQ(outputs.size(), 1U);
+ CHECK_EQ(req.size(), 1U);
+ CHECK_EQ(req[0], kWriteTo) << "intgemm only overwrites";
+
+ const TBlob &in = inputs.front();
+ const TBlob &out = outputs.front();
+ CHECK_EQ(out.type_flag_, mshadow::kInt8);
+ CHECK(in.CheckContiguous());
+ CHECK(out.CheckContiguous());
+ size_t B_cols = in.shape_.ProdShape(0, in.shape_.ndim() - 1);
+ size_t inner = in.shape_[in.shape_.ndim() - 1];
+ CHECK_EQ(inner % ::intgemm::Int8::tile_info.b_rows, 0) <<
+ "intgemm requires the inner dimension be a multiple of " << ::intgemm::Int8::tile_info.b_rows;
+ CHECK_EQ(B_cols % ::intgemm::Int8::tile_info.b_cols, 0) <<
+ "intgemm requires the output dimension (the product of all but the last dimension of the "
+ "weight matrix) to be a multiple of " << ::intgemm::Int8::tile_info.b_cols << ".";
+
+ int8_t *quantB = out.dptr<int8_t>();
+ CHECK(in.type_flag_ == mshadow::kFloat32 || in.type_flag_ == mshadow::kInt8) <<
+ "Expected either 32-bit values to be quantized or 8-bit values to rearrange.";
+ if (in.type_flag_ == mshadow::kInt8) {
+ const int8_t *B = in.dptr<int8_t>();
+ ::intgemm::Int8::PrepareBQuantizedTransposed(B, quantB, inner, B_cols);
+ } else if (in.type_flag_ == mshadow::kFloat32) {
+ const float *B = in.dptr<float>();
+ // TODO(kpuatamazon): eliminate transpose here with https://github.com/kpu/intgemm/pull/56
+ intgemm::AlignedVector<float> B_transpose(inner * B_cols);
+ for (size_t i = 0; i < inner; ++i) {
+ for (size_t j = 0; j < B_cols; ++j) {
+ B_transpose[i * B_cols + j] = B[i + inner * j];
+ }
+ }
+ ::intgemm::Int8::PrepareB(
+ B_transpose.begin(),
+ quantB,
+ 127.0 / *inputs[1].dptr<float>(),
+ inner,
+ B_cols);
+ }
+}
+
+NNVM_REGISTER_OP(_contrib_intgemm_prepare_weight)
+.describe(R"code(This operator converts a weight matrix in column-major format to intgemm's internal fast representation of weight matrices. MXNet customarily stores weight matrices in column-major (transposed) format. This operator is not meant to be fast; it is meant to be run offline to quantize a model.
Review comment:
I'm interested in your suggestions on how best to do this. Keep in mind we do have a use case for storing 8-bit quantized models on disk.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663647064
Jenkins CI successfully triggered : [unix-gpu, centos-gpu, unix-cpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-664270598
@mxnet-bot run ci [unix-cpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r476832030
##########
File path: CMakeLists.txt
##########
@@ -64,6 +64,13 @@ if(USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND (CMAKE_HOST_SYSTEM_PR
else()
option(USE_MKLDNN "Build with MKL-DNN support" OFF)
endif()
+#gcc 4 doesn't support AVX2 and SSSE3 support doesn't work with target attributes so ban gcc < 5 from intgemm.
+if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT CMAKE_CROSSCOMPILING) AND
+ ((NOT CMAKE_COMPILER_IS_GNUCC) OR (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 5.0)))
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" ON)
+else()
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" OFF)
+endif()
Review comment:
gcc7 is already required. You don't need any special handling here.
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
`add_subdirectory` makes available all exported targets. You should only need a call à la `target_link_libraries(mxnet PRIVATE intgemm)`
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
Do we expect a large number of users to compile with intgemm? If not, why force all users to download intgemm instead of making it optional?
```
FetchContent_Declare(
intgemm
GIT_REPOSITORY https://github.com/kpu/intgemm.git
GIT_TAG 77095a0cd5da8a22c4e7ed77588b527d3ee4ada9
)
FetchContent_GetProperties(catch)
if(NOT catch_POPULATED)
FetchContent_Populate(catch)
add_subdirectory(${catch_SOURCE_DIR} ${catch_BINARY_DIR})
endif()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646073
Undefined action detected.
Permissible actions are : run ci [all], run ci [job1, job2]
Example : @mxnet-bot run ci [all]
Example : @mxnet-bot run ci [centos-cpu, clang]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479367727
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
The aim is that Sockeye will be able to use `pip` installed `mxnet`. Given the withdrawal of #18904, there isn't a nice way to add to the `pip` version. Is download or submodule preferred?
cc @fhieber
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpu commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpu commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-638973973
@ChaiBapchya Sure I'll sync master on Monday.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ChaiBapchya commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-656784891
Sure. Let's get the tests to pass.
@kpuatamazon Any progress on mac test? Any way I can help?
Also for sanity pipeline to pass, lets make the intgemm submodule compatible with RAT license.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479358914
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
Added `EXCLUDE_FROM_ALL`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r464297908
##########
File path: .gitmodules
##########
@@ -25,3 +25,6 @@
[submodule "3rdparty/nvidia_cub"]
path = 3rdparty/nvidia_cub
url = https://github.com/NVlabs/cub.git
+[submodule "3rdparty/intgemm"]
Review comment:
MIT license should be fine right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r480041199
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
Back to fetching for now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479412917
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
33ad78208c911a5e9d770416266b0d3510767213 now uses `target_link_libraries(mxnet PRIVATE intgemm)`
Regarding `INTGEMM_OPERATOR_SOURCE`, that's not adding stuff, it's removing the MXNet operator implementations (which were caught by the glob) if the underlying library isn't compiled in.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ChaiBapchya commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-638973103
@kpuatamazon Can we resolve the conflicting files? Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r480032723
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
Header path pollution added in e6ddba887f0e96b8be288b3d3044c9a7e1531fca, now in intgemm/ directory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479439896
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
It's bad practice to modify the gobal list of default include directories. If the intgemm CMakeLists.txt is correct, these two lines are not needed as `target_link_libraries(mxnet PRIVATE intgemm)` will handle the includes automatically.
##########
File path: CMakeLists.txt
##########
@@ -64,6 +64,11 @@ if(USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND (CMAKE_HOST_SYSTEM_PR
else()
option(USE_MKLDNN "Build with MKL-DNN support" OFF)
endif()
+if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT CMAKE_CROSSCOMPILING))
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" ON)
+else()
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" OFF)
+endif()
Review comment:
If `USE_INTGEMM` is only supported on x86, you should use `cmake_dependent_option`. If it's supported but not recommended on other architectures, your current approach is fine, though you may want to update the docstring to clarify.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] eric-haibin-lin commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
eric-haibin-lin commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r463160220
##########
File path: .gitmodules
##########
@@ -25,3 +25,6 @@
[submodule "3rdparty/nvidia_cub"]
path = 3rdparty/nvidia_cub
url = https://github.com/NVlabs/cub.git
+[submodule "3rdparty/intgemm"]
Review comment:
@szha anything we should pay attention to w.r.t. licensing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646058
@mxnet-bot run ci centos-gpu, unix-cpu, unix-gpu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-683749858
@mxnet-bot run ci [unix-gpu]
Looks like an unrelated test failure
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r476846045
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
Do we expect a large number of users to compile with intgemm? If not, why force all users to download intgemm instead of making it optional?
```
FetchContent_Declare(
intgemm
GIT_REPOSITORY https://github.com/kpu/intgemm.git
GIT_TAG 77095a0cd5da8a22c4e7ed77588b527d3ee4ada9
)
FetchContent_GetProperties(intgemm)
if(NOT intgemm_POPULATED)
FetchContent_Populate(intgemm)
add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR})
endif()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-677828699
cc @leezu to review the build logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-683909401
Thank you @kpuatamazon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-683749888
Jenkins CI successfully triggered : [unix-gpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479501831
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> Currently you're hardcoding the `#include "../../../../3rdparty/intgemm/intgemm.h"` path inside the MXNet source files.
This is common practice:
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/common/tensor_inspector.h#L35
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cc#L25
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cu#L27
> Essentially you need to update the intgemm cmakelists file to ensure it encapsulates the headers as part of the intgemm target.
Were I to do that, it would unnecessarily pollute the include paths of software that references the file by its path in the source/submodule tree.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r471582693
##########
File path: .gitmodules
##########
@@ -25,3 +25,6 @@
[submodule "3rdparty/nvidia_cub"]
path = 3rdparty/nvidia_cub
url = https://github.com/NVlabs/cub.git
+[submodule "3rdparty/intgemm"]
Review comment:
Done in 38424013c6e75724b8f42cf83476174a1bc33c23
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mjdenkowski commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mjdenkowski commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-656298730
What do we think about merging this PR when all of the tests pass?
We've seen thorough comparisons showing a clear speedup from intgemm. Kenneth has also explored using the custom operator API, but there appears to be an unresolvable performance overhead when calling external operators. Including the intgemm operators directly in MXNet will allow all users to get the maximum performance benefit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-657550007
The mac error turns out to be a clang bug in that target attributes don't fully work as well as the command line.
Here's `example.cc`:
```C++
#include <immintrin.h>
__attribute__ ((target ("avx2"))) void Foo(__m256i sum0) {
asm("" : [sum0] "+x" (sum0));
}
```
It compiles fine with `-mavx2`:
```
clang++-7 -mavx2 -O3 -c example.cc
```
Then doesn't compile without `-mavx2`:
```
clang++-7 -O3 -c example.cc
example.cc:3:25: error: invalid output size for constraint '+x'
asm("" : [sum0] "+x" (sum0));
^
1 error generated.
```
Even though target attributes are supposed to act like they appeared on the command line. Ugh, I'll get started on a workaround.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479501831
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> Currently you're hardcoding the `#include "../../../../3rdparty/intgemm/intgemm.h"` path inside the MXNet source files.
This is common practice:
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/common/tensor_inspector.h#L35
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cc#L25
https://github.com/apache/incubator-mxnet/blob/a9ca6600384265fcf25ce2993019f9542540fda1/src/operator/nn/ctc_loss.cu#L27
> Essentially you need to update the intgemm cmakelists file to ensure it encapsulates the headers as part of the intgemm target.
Were I to do that, it would unnecessarily pollute the include paths of software that references the file by its path in the source/submodule tree. And it would potentially introduce header name conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r460226010
##########
File path: src/operator/contrib/intgemm/prepare_data_op.cc
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file prepare_data_op.cc
+ * \brief Converts data aka A matrices (typically activations) to intgemm's
+ * representation for A in C=AB. This just quantizes to int8 and bans -128.
+ * The only difference from Quantize/QuantizeV2 is that it bans -128.
Review comment:
this sounds more like a utility function than an operator, as I'm not sure if it makes sense to ever expose the intgemm specific representation in the frontend.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479484233
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
I've pushed a submodule version without any `include_directories`. If you want to use `FetchContent` then the directory is variable and there needs to be some mechanism for the MXNet code to find `intgemm`. And the need for such a mechanism is a peculiarity of MXNet.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-677895152
Otherwise LGTM. I reviewed tests and and op implementation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663645689
Undefined action detected.
Permissible actions are : run ci [all], run ci [job1, job2]
Example : @mxnet-bot run ci [all]
Example : @mxnet-bot run ci [centos-cpu, clang]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479480270
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> However, include_directories(${intgemm_SOURCE_DIR}) remains. intgemm itself has entirely relative header paths and users do not need to -I the source directory to use it, so it would be incorrect to add that to the library.
If users in general do not neet to `-I` the source directory, why does the particular user mxnet need to do so? Why not associate the public headers with the intgemm `target_include_directories(intgemm PUBLIC ${CMAKE_CURRENT_LIST_DIR})`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479358134
##########
File path: CMakeLists.txt
##########
@@ -64,6 +64,13 @@ if(USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND (CMAKE_HOST_SYSTEM_PR
else()
option(USE_MKLDNN "Build with MKL-DNN support" OFF)
endif()
+#gcc 4 doesn't support AVX2 and SSSE3 support doesn't work with target attributes so ban gcc < 5 from intgemm.
+if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT CMAKE_CROSSCOMPILING) AND
+ ((NOT CMAKE_COMPILER_IS_GNUCC) OR (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 5.0)))
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" ON)
+else()
+ option(USE_INTGEMM "Build with x86 intgemm library for low-precision multiplication" OFF)
+endif()
Review comment:
d03342adbe0d68528b3e8e26c8517b4fbdb0eec9
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r464606382
##########
File path: .gitmodules
##########
@@ -25,3 +25,6 @@
[submodule "3rdparty/nvidia_cub"]
path = 3rdparty/nvidia_cub
url = https://github.com/NVlabs/cub.git
+[submodule "3rdparty/intgemm"]
Review comment:
yes, MIT license is ok. make sure we add the corresponding text in the LICENSE file in this repo.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-664270742
Jenkins CI successfully triggered : [unix-cpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r480041535
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE "src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
I take your :+1: emoji to mean this part is resolved?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479539243
##########
File path: CMakeLists.txt
##########
@@ -278,6 +283,25 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ include(FetchContent)
+ FetchContent_Declare(
+ intgemm
+ GIT_REPOSITORY https://github.com/kpu/intgemm.git
+ GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023
+ )
+ FetchContent_GetProperties(intgemm)
+ if(NOT intgemm_POPULATED)
+ FetchContent_Populate(intgemm)
+ add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL)
+ endif()
+ include_directories(${intgemm_SOURCE_DIR})
+ #intgemm generates a config header based on AVX512 support in the compiler.
+ include_directories(${intgemm_BINARY_DIR})
Review comment:
> This is common practice:
I think the MXNet source is not always a good reference for best practices :)
> Were I to do that, it would unnecessarily pollute the include paths of software that references the file by its path in the source/submodule tree.
Having files hardcode the direct path in the submodule tree limits the use of intgemm. For example, consider users that adopt conan.io or linux distributions that prefer to package intgemm separately from MXNet.
> And it would potentially introduce header name conflicts.
It's common practice to put the public header files into `include/intgemm` instead of the top level directory. This will avoid any name conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on a change in pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r475807106
##########
File path: tests/python/unittest/test_contrib_intgemm.py
##########
@@ -0,0 +1,205 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
Done in 29cc970
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] kpuatamazon commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
kpuatamazon commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-674908510
> Test is required for any code change.
@szha Tests are now in and passing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu merged pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17559: [MXNET-1446] Quantization: intgemm matrix multiply wrappers
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#issuecomment-663646835
Undefined action detected.
Permissible actions are : run ci [all], run ci [job1, job2]
Example : @mxnet-bot run ci [all]
Example : @mxnet-bot run ci [centos-cpu, clang]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org