New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Arith] linear system and equation solver #5171

Merged

tqchen merged 7 commits into apache:master from yzhliu:linear_solver

Apr 10, 2020

Member

yzhliu commented Mar 30, 2020

Co-authored-by: Sergei Grechanik sergei.grechanik+h@gmail.com

Linear system data structures and linear equations solver (via Smith Normal Form)

this is part of tensor level autodiff. will be used by #5121

Originally implemented by @sgrechanik-h in #2634 I modified a bit to make it more mathematical. @sgrechanik-h please help to check if the math is correct and whether SuperSimplify is required (I use tir::Simplify instead in this PR, I think it is only required in condition lift)

@sgrechanik-h @tqchen @MarisaKirisame @junrushao1994 @hzfan Please review

yzhliu added the status: need review label

yzhliu force-pushed the linear_solver branch from 0a75caf to 2b0ad1a Compare

March 30, 2020 08:05

Member

tqchen commented Mar 30, 2020

NOTE: please try to rewrite to directly use the auxiliary class arith::Analyzer instead of the Simplify function, so that the temp data structure can be created only once

tqchen mentioned this pull request

[TVM] Zero elimination #2634

Closed

yzhliu force-pushed the linear_solver branch from 2b0ad1a to 5a9c022 Compare

March 31, 2020 23:19

Member Author

yzhliu commented Mar 31, 2020

@tqchen changed to use arith::Analyzer

sergei-grechanik reviewed

View reviewed changes

include/tvm/arith/linear_system.h Outdated

+              /*!
+               * \brief Represent a linear system including variables, their ranges and
+               *        the linear relations between them (either equations or inequalities)

Contributor

sergei-grechanik Apr 1, 2020

I think it's worth mentioning that the system is integer.

Also I'm not sure if LinearSystem is a good name, because it may be useful to have non-linear (in)equalities in relations (which doesn't break the algorithms that work on the linear part).

Member Author

yzhliu Apr 2, 2020 •

edited

Loading

good suggestion. how about Formulas, Constraints, RelationSet, IntRelationSet, or InequalitySet (if we make term "inequality" to also include equations)? @sergei-grechanik

Contributor

sergei-grechanik Apr 3, 2020

IntegerSet or IntegerSystem maybe?

Member Author

yzhliu Apr 3, 2020

sounds good. There's IntSet in tvm, so I'll do IntegerSystem.

Member Author

yzhliu Apr 3, 2020

think it a bit more, should we use a more general name? as in solver, we will check the relation and ignore the non-integer equations, which implies it is valid to contain non-integer relations inside. @sergei-grechanik

Contributor

sergei-grechanik Apr 4, 2020

If variables are integer, I think it's still ok to call it IntegerSystem. But if we allow non-integer variables (I doubt it is a good idea), then I don't know, Set and System sound too unspecific, Constraints may be ok.

include/tvm/arith/linear_system.h Outdated Show resolved Hide resolved

tests/python/unittest/test_arith_solve_linear_system.py Outdated Show resolved Hide resolved

Contributor

sergei-grechanik commented Apr 1, 2020

About SuperSimplify: it was a combination of the canonical simplifier and the rewriting simplifier that worked best for autodiff (I actually ran it with different combinations and rewrite->canonical->rewrite turned out to be the best, although canonical->rewrite was good too). I think currently the default Simplify function is rewrite->canonical, not sure if this order has a good justification.

Member Author

yzhliu commented Apr 1, 2020

@sergei-grechanik do you have example that we have to end with rewrite?

MarisaKirisame approved these changes

View reviewed changes

Contributor

MarisaKirisame left a comment

LGTM, although i have essentially zero expertise in numerical algorithm, so I didnt really understand what is going on.

src/arith/linear_system.cc Outdated

+                node->variables = std::move(variables);
+                node->ranges = std::move(ranges);
+                node->relations = std::move(relations);
+                data_ = std::move(node);

Contributor

MarisaKirisame Apr 1, 2020

what is data_?

Member Author

yzhliu Apr 3, 2020

it's ObjectRef->data_ whose type is ObjectPtr

src/arith/solve_linear_equation.cc Show resolved Hide resolved

Member

tqchen commented Apr 1, 2020

cc @Hzfengsy @spectrometerHBH @merrymercy please also take a look if you have time, should be relevant to your areas

Contributor

sergei-grechanik commented Apr 1, 2020

@yzhliu Unfortunately I don't have an example. It's written in my notes that "we should end with rw because it factors multipliers out". If it's only that, it's probably not that important.

hzfan reviewed

View reviewed changes

src/arith/solve_linear_equation.cc Show resolved Hide resolved

src/arith/solve_linear_equation.cc

+                // Transform formulas into rows of the matrix
+                // S_{mxn} V^{-1}_{nxn} x_{nx1} = U y, in which n is # of variables
+                // here we initialize S_{mxn} to be A, U to be identity matrix.

Contributor

hzfan Apr 2, 2020

V to be identity matrix?

Member Author

yzhliu Apr 3, 2020

V was described above. here what I mean is
U^{-1} S V^{-1} x = y = [coefs] => S V^{-1} x = U [coefs]
which implies U is identity.

src/arith/solve_linear_equation.cc Outdated Show resolved Hide resolved

yzhliu force-pushed the linear_solver branch 2 times, most recently from c58057b to c2678f8 Compare

April 5, 2020 06:05

Member Author

yzhliu commented Apr 5, 2020

@sergei-grechanik I rename it to IntConstraints, and also add checker to ensure the variables to be integers. please take a review again.

yzhliu force-pushed the linear_solver branch from c2678f8 to 3ccd472 Compare

April 5, 2020 06:11

sergei-grechanik reviewed

View reviewed changes

tests/python/unittest/test_arith_solve_linear_system.py Outdated Show resolved Hide resolved

tests/python/unittest/test_arith_solve_linear_system.py Show resolved Hide resolved

tests/python/unittest/test_arith_solve_linear_system.py Show resolved Hide resolved

src/arith/solve_linear_equation.cc Show resolved Hide resolved

tests/python/unittest/test_arith_solve_linear_system.py Outdated Show resolved Hide resolved

tests/python/unittest/test_arith_solve_linear_system.py Show resolved Hide resolved

sergei-grechanik reviewed

View reviewed changes

include/tvm/arith/int_solver.h

+               * \param V  an identity matrix, it will be modified to V_{nxn}
+               * \param x  the x in A x = y. it will be modified to V^{-1}_{nxn} x_{nx1}
+               * \param y  the y in A x = y. it will be modified to U_{mxm} y_{mx1}
+               */

Contributor

sergei-grechanik Apr 8, 2020

I was thinking about this and came to conclusion that computing the proper Smith normal form may improve stability of automatic differentiation (I mean generating the same gradient code for slightly different but equivalent input code). However, I'm not sure if it's worth it (and not in this PR of course).

Member Author

yzhliu Apr 8, 2020

I see. I'll put a todo here.

yzhliu force-pushed the linear_solver branch 2 times, most recently from 3f7424e to 0010a64 Compare

April 8, 2020 07:27

sergei-grechanik reviewed

View reviewed changes

tests/python/unittest/test_arith_solve_linear_system.py Outdated Show resolved Hide resolved

sergei-grechanik reviewed

View reviewed changes

python/tvm/arith/int_solver.py Outdated Show resolved Hide resolved

sergei-grechanik reviewed

View reviewed changes

include/tvm/arith/int_solver.h Show resolved Hide resolved

yzhliu and others added 6 commits

April 9, 2020 17:17


          [arith] linear system and equation solver

0aa5115

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>


          avoid constructing analyzer every time

42317d9


          generate random test cases and address comments

ff42ed1

Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>


          rename linear_system to int_constraints


          add comments and use random seed

8cac9e3


          message for reporting failure with seed

2d89fef


          add SEqualReduce to IntConstraints; allow variables & ranges to be None

dc76eac

yzhliu force-pushed the linear_solver branch from 0010a64 to dc76eac Compare

April 10, 2020 00:17

sergei-grechanik approved these changes

View reviewed changes

Contributor

sergei-grechanik left a comment

Ok, looks good to me

tqchen approved these changes

View reviewed changes

tqchen merged commit e21f268 into apache:master

tqchen added status: accepted and removed status: need review labels

Member

tqchen commented Apr 10, 2020

Thanks @yzhliu @sgrechanik-h @hzfan @MarisaKirisame

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request


          [Arith] linear system and equation solver (apache#5171)

* [arith] linear system and equation solver

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>

* avoid constructing analyzer every time

* generate random test cases and address comments

Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

* rename linear_system to int_constraints

* add comments and use random seed

* message for reporting failure with seed

* add SEqualReduce to IntConstraints; allow variables & ranges to be None

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>
Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

zhiics pushed a commit to neo-ai/tvm that referenced this pull request


          [Arith] linear system and equation solver (apache#5171)

bde2419

* [arith] linear system and equation solver

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>

* avoid constructing analyzer every time

* generate random test cases and address comments

Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

* rename linear_system to int_constraints

* add comments and use random seed

* message for reporting failure with seed

* add SEqualReduce to IntConstraints; allow variables & ranges to be None

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>
Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

dpankratz pushed a commit to dpankratz/incubator-tvm that referenced this pull request


          [Arith] linear system and equation solver (apache#5171)

1ee5937

* [arith] linear system and equation solver

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>

* avoid constructing analyzer every time

* generate random test cases and address comments

Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

* rename linear_system to int_constraints

* add comments and use random seed

* message for reporting failure with seed

* add SEqualReduce to IntConstraints; allow variables & ranges to be None

Co-authored-by: Sergei Grechanik <sergei.grechanik+h@gmail.com>
Co-authored-by: Sergei Grechanik <sergei.grechanik@gmail.com>

ZihengJiang mentioned this pull request

TVM v0.7 Release Note Candidate #6486

Closed

monklof pushed a commit to monklof/incubator-tvm that referenced this pull request


          Merge pull request apache#3 in DATA/tvm from ~WANGXIN66/tvm_clone_fro…

f6767fd

…m_data:master to master

* commit 'cd0d52daa6942bdafa9363ff6cfa3d25fcd5b8d6': (824 commits)
  [Intrinsic] Add log1p, ldexp, atan2, hypot, nextafter, copysign (apache#5312)
  [Rust][CI] Restore Rust CI (apache#5137)
  Remove PrimExpr from String (apache#5311)
  [Requantize] Cleanup and Optimize Lowering (apache#5286)
  [IR][TRANSFORM] Enable CopyOnWrite for passes. (apache#5309)
  [PYTORCH]Abs, Arange, Softplus ops (apache#5295)
  [LLVM] Fix generation of LLVM intrinsics (apache#5282)
  [BYOC] Add example of Composite + Annotate for DNNL fused op (apache#5272)
  [Frontend][TensorFlow]Improve TensorFlow Static Shape Tensor Array (apache#5243)
  [RUNTIME] Introduce RValue reference(move) support to TypedPackedFunc (apache#5271)
  [RELAY][FRONTEND][CAFFE2] add Mul and ConvTranspose operator (apache#5302)
  [BYOC] Refine AnnotateTarget and MergeCompilerRegion Passes (apache#5277)
  [CI] Fix the hexagon string (apache#5304)
  [Arith] linear system and equation solver (apache#5171)
  [PYTORCH]Repeat, Reciprocal & Reshape Op support (apache#5280)
  [FRONTEND][TENSORFLOW] Fix gather_nd indices (apache#5279)
  Update device_annotation.cc (apache#5291)
  [REFACTOR][IR] Move to runtime::String (apache#5276)
  [NDArray] Set shape_ in NDArray::FromDLPack (apache#5301)
  [RUNTIME] Initial implementation of Hexagon runtime support (apache#5252)
  ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment