You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "cbalint13 (via GitHub)" <gi...@apache.org> on 2023/04/03 18:40:52 UTC
[GitHub] [tvm] cbalint13 opened a new pull request, #14468: Binarize labels for the new xgboost >= 1.7.5 behaviour
cbalint13 opened a new pull request, #14468:
URL: https://github.com/apache/tvm/pull/14468
This PR fix the latest xgboost >= 1.7.5 behaviour requiring binarized labels.
* The strict behaviour was introduced by https://github.com/dmlc/xgboost/pull/8931
* A possible solution (also [suggested](https://github.com/dmlc/xgboost/pull/9007#issuecomment-1494739265)) would be to binarize by threshold at the evaluation calls.
* This is a continuation on https://github.com/apache/tvm/pull/14036 xgboost
This address both ```autotune``` and ```autoscheduler```.
**Note:**
Unsure about TVM overall tunner impact, but we can introduce more sophisticated way of measuring AP like [PASCAL evenly spaced one](https://en.wikipedia.org/wiki/Evaluation_measures_(information_retrieval)#Average_precision), the advantages are unclear and would require extensive comparative tests.
---
The errors cought on TVM **autotune** process:
```
File "/usr/lib64/python3.11/site-packages/tvm/autotvm/tuner/xgboost_cost_model.py"
, line 538, in after_iteration
bst_eval = model.eval_set(self.evals, epoch, feval)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/site-packages/xgboost/core.py", line 1995, in eval_set
_check_call(
File "/usr/lib64/python3.11/site-packages/xgboost/core.py", line 270, in _check_call
raise XGBoostError(py_str(_LIB.XGBGetLastError()))
xgboost.core.XGBoostError: [15:59:59] /builddir/build/BUILD/xgboost/src/common/ranking_utils.h:378:
Check failed: is_binary: MAP can only be used with binary labels.
```
---
Cc @Sunny-Island , @zxybazh , @junrushao , @vinx13 , please help with the review.
Thanks,
~Cristian.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] FrozenGene commented on a diff in pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "FrozenGene (via GitHub)" <gi...@apache.org>.
FrozenGene commented on code in PR #14468:
URL: https://github.com/apache/tvm/pull/14468#discussion_r1161498222
##########
vta/scripts/tune_resnet.py:
##########
@@ -184,7 +184,7 @@ def tune_tasks(
# create tuner
if tuner == "xgb" or tuner == "xgb-rank":
Review Comment:
```tuner == "xgb-rank"``` should be changed too.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1497916664
@tqchen ,
> @cbalint13 Yes, let us update and to change all to reg.
* Updated all tutorials, apps and tests from any explicit "rank" to "reg".
> cc @junrushao to double check cases in MetaSchedule.
* Double check is welcome, as quick info, none of my metascheduler test failed, xgboost would print:
```
xgboost.core.XGBoostError: [15:59:59] /builddir/build/BUILD/xgboost/src/common/ranking_utils.h:378:
Check failed: is_binary: MAP can only be used with binary labels.
```
* Used xgboost 20230403 git hash 15e073ca .
> Might be useful to use reg:logistic, if the output is scaled into [0, 1]
My thought on the newly introduced ```type_loss="rank-binary"```:
* Is **alternative** to the original ```rank``` and **booth have**: ```("objective": "rank:pairwise")```
* It binarize (within a cloned copy) the labels only for the xgboost eval step (being xgboost>=2 compatible).
* The dynamics inside TVM further steps still remained with full continuous values.
I would leave it as described, if would like we can create another one ```type_loss="reg-binary" w/ reg:logistic```.
For the case reg:logistic the naming "rank-binary" loose the "rank"-ness, so it would be just another "reg" class type.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tvm-bot commented on pull request #14468: Binarize labels for the new xgboost >= 1.7.5 behaviour
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1494799897
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to auto-tag found, no teams are specified in PR title <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on a diff in pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #14468:
URL: https://github.com/apache/tvm/pull/14468#discussion_r1161596806
##########
vta/scripts/tune_resnet.py:
##########
@@ -184,7 +184,7 @@ def tune_tasks(
# create tuner
if tuner == "xgb" or tuner == "xgb-rank":
Review Comment:
@FrozenGene ,
Uptade
> @FrozenGene
>
> > `tuner == "xgb-rank"` should be changed too.
>
> Thanks for the observation !
>
> * fixed and exposed properly all such cases.
> * exposed / updated tmvc with extended options coverage.
> * extended the demo lists (taken from original `tune_relay_arm.py` for completeness)
Plus:
* leaved "rank" types in the list for compatibility with xgboost < 2.0.0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoTVM] New rank-binary loss_type labels for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496752658
@tqchen ,
* Introduced a new ```rank-binary``` loss_type along with current ```rank``` and ```reg```.
> I see, i think we should report error if binarization is needed, since the original intention was continuous prediction.
* If loss_type is ```rank``` and xgboost >= 2.0.0 will report an error with explicit suggestion to use ```rank-binary``` instead.
> I know it might still work OK, but that was not the intention of the cost predictor.
* All remains as it was, it is user's choice (informed by tvm) to switch over with ```rank-binary``` or downgrade xgboost.
> Would be good to visit the default choice, i think if ranking is not possible, reg:logistic would be another good choice usually
* I won't touch this, all the described above would offer user a informed alternative.
Let me know if still need changes or more polishing, I stop here for now.
In future, will keep track on xgboost upcoming ```custom gain function``` , we may restore ```rank``` continuous behaviour.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen merged pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen merged PR #14468:
URL: https://github.com/apache/tvm/pull/14468
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoScheduler][AutoTVM] Binarize labels for the new xgboost >= 1.7.5 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496441262
@tqchen , additionally to my response to the request in the previous message:
> > Likely we might want to move away from the MAP metric, and use other metric instead, either regression metric or pair-wise ranking.
>
> * Apparently this proposal works, tunning finds good kernels, but the real impact is hard to measure (on personal side).
>
> Another quick idea for now is to add condition of binarization to xgboost >=1.7.5 version, keeping the old behaviour.
* If binarization is too **steep** , a simple (but recursive) way would be the [PASCAL](https://en.wikipedia.org/wiki/Evaluation_measures_(information_retrieval)#Average_precision) trick to avg/split into 11 levels:
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496812611
we still need to make sure the default `loss_type` now updates to regression loss
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoTVM] Binarize labels for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496600164
@tqchen ,
> I think in this case we should change ranking loss to regression loss, use logistic regression so the values can still be used. binarization causes too much info loss
Updates:
1. The autoschedule (ansor) is not affected at all.
2. The autotune ```reg (reg:linear)``` loss_type is not affected.
3. Only autotune with ```rank (rank:pairwise)``` loss_type``` is affected.
4. Only xgboost > 2.0.0-dev (as it presents itself via py API).
I updated this PR code to do binarization **only in case**:
- check if xgboost > 2
- check if rank:pairwise
Updated here the code, the title, the first comment (barred out any erroneous info).
I could imagine to leave this for **autotune** in a prepearing for what will be xgb > 2.0.0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoScheduler][AutoTVM] Binarize labels for the new xgboost >= 1.7.5 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496426370
@tqchen
> Given that we are doing cost model. I am not sure if binarization is the best approach here.
* In a short discussion here this was suggested: https://github.com/dmlc/xgboost/pull/9007#issuecomment-1494739265
* Looking through [changes](https://github.com/dmlc/xgboost/pull/8931) , the old behaviour also clamped somehow the values (not clear for me if to pure binary).
> Can you dump out the labels and check the current assigned behavior?
* Sure, attached is a small script + dmatrix dump: [tvm-xgboost-dmatrix.zip](https://github.com/apache/tvm/files/11151803/tvm-xgboost-dmatrix.zip) with [results.txt](https://github.com/apache/tvm/files/11151857/results.txt)
* This was captured from a real tvm autotunning process targeting a rk3399 opencl device.
>
> Likely we might want to move away from the MAP metric, and use other metric instead, either regression metric or pair-wise ranking.
* Apparently this proposal works, tunning finds good kernels, but the real impact is hard to measure (on personal side).
Another quick idea for now is to add condition of binarization to xgboost >=1.7.5 version, keeping the old behaviour.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #14468: [AutoScheduler][AutoTVM] Binarize labels for the new xgboost >= 1.7.5 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496441932
I think in this case we should change ranking loss to regression loss, use logistic regression so the values can still be used. binarization causes too much info loss
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1497612710
@cbalint13 Yes, let us update and to change all to reg. cc @junrushao to double check cases in MetaSchedule.
Might be useful to use reg:logistic, if the output is scaled into [0, 1]
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on a diff in pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #14468:
URL: https://github.com/apache/tvm/pull/14468#discussion_r1161580129
##########
vta/scripts/tune_resnet.py:
##########
@@ -184,7 +184,7 @@ def tune_tasks(
# create tuner
if tuner == "xgb" or tuner == "xgb-rank":
Review Comment:
@FrozenGene
> `tuner == "xgb-rank"` should be changed too.
Thanks for the observation !
* fixed and exposed properly all such cases.
* exposed / updated tmvc with extended options coverage.
* extended the demo lists (taken from original ```tune_relay_arm.py``` for completeness)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #14468: [AutoScheduler][AutoTVM] Binarize labels for the new xgboost >= 1.7.5 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1495997462
Given that we are doing cost model. I am not sure if binarization is the best approach here. Can you dump out the labels and check the current assigned behavior?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #14468: [AutoTVM] Binarize labels for the new xgboost >= 2.0.0 behaviour
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1496704096
I see, i think we should report error if binarization is needed, since the original intention was continuous prediction.
I know it might still work OK, but that was not the intention of the cost predictor.
Would be good to visit the default choice, i think if ranking is not possible, reg:logistic would be another good choice usually
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1497093298
@tqchen ,
> Thanks @cbalint13 ! we still need to make sure the default `loss_type` now updates to regression loss, mainly because rank-binary do not fit into the customized loss models.
* Changed to "reg" all class init occurrences.
* Lowered test case rmse for the new "reg" default.
There are still tutorials / applications that use explicit "rank", would like to change 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cbalint13 commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1499637329
@tqchen , @junrushao
In continuation of previous comment, I also attach here some test result.
Comparative test confirms that ```rank-binary``` (binarized only at eval step) behaves identically with ```rank``` (original):
loss_type="rank-binary" (xgboost-2.0.0-dev 20230403 git hash 15e073ca)
[Task 1/54] (conv2d_nchw_spatial_pack.mali) {17.75 GFLOPS / #4912 records} SKIP
[Task 2/54] (conv2d_nchw_spatial_pack.mali) {40.74 GFLOPS / #1040 records} SKIP
[Task 3/54] (conv2d_nchw_spatial_pack.mali) {19.63 GFLOPS / #2032 records} SKIP
loss_type="rank" (xgboost-1.7.5 20230328 git hash 21d95f3d)
[Task 1/54] (conv2d_nchw_spatial_pack.mali) {11.71 GFLOPS / #1680 records} SKIP
[Task 2/54] (conv2d_nchw_spatial_pack.mali) {26.17 GFLOPS / #1024 records} SKIP
[Task 3/54] (conv2d_nchw_spatial_pack.mali) {13.15 GFLOPS / #1040 records} SKIP
Note:
* In the case of rank-binary there was more steps (bit prologed / see amount of records) hence bit better results.
* The tuned network was first three layers of yolov8s using float16 for half a day on a rk3399 board (nanopc-t4).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] zxybazh commented on pull request #14468: [AutoTVM] New rank-binary loss_type for the new xgboost >= 2.0.0 behaviour
Posted by "zxybazh (via GitHub)" <gi...@apache.org>.
zxybazh commented on PR #14468:
URL: https://github.com/apache/tvm/pull/14468#issuecomment-1500677347
@tvm-bot rerun
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org