Skip to content
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.

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

Linear Algebra API & Parser #364

Open
wants to merge 70 commits into
base: master
Choose a base branch
from
Open

Conversation

mattlkf
Copy link

@mattlkf mattlkf commented Jan 14, 2021

This adds a new front-end for manipulating 1D/2D tensors (vectors, matrices) with linear algebra notation. Matrix/Vector multiplication, transpose and element-wise ops are supported.

Using the "-linalg" flag with ./bin/taco turns on parsing of linear algebra expressions. Shapes of the linear algebra variables can be (optionally) specified via the "-k=:" flag. If not specified, variable names starting with capital letters are assumed to be matrices.

Example usage:
./bin/taco "A=B*C + transpose(x * transpose(y))" -linalg

Besides the parser, an API that provides Matrix and Vector classes has been added. Tests demonstrating its use are in tests/tests-linalg.cpp.

Slides from talk given at TACO Weekly (1/6/2021) here:
https://docs.google.com/presentation/d/1zX9mUt9iqvwK6vVuakI882s7mc8-XNbUmqXRIw_HI5U/edit?usp=sharing

weiya711 and others added 30 commits October 30, 2020 16:06
weiya711 and others added 26 commits December 15, 2020 11:04
Fixes assertion failure in LinalgBase::operator= (compiled in debug
mode)
Datatype getDataType() const;
int getOrder() const;
bool isColVector() const;
void setColVector(bool) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the intended use case for this method? I don't see this invoked anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephenchouca This setColVector() function is for when the user defines a vector as row (or col) in the constructor, but then later realizes the colVector flag needs to be changed. It's not used anywhere but is provided as part of the user LA API


Datatype getDataType() const;
int getOrder() const;
bool isColVector() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should this return for scalars or matrices?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns false for vectors and matrices always. Do you have a better suggestion of how to do this?

public:
explicit Matrix(std::string name);

Matrix(std::string name, size_t dim1, size_t dim2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dim1 and dim2 should probably just be named rows and cols?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change this

}

// Read method
CType at(int coord_x, int coord_y);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why coord_x and coord_y are used here when i and j are used for operator(). I think it'd make more sense to use one or the other consistently.

IndexStmt indexAssignment;

int idxcount = 0;
std::vector<std::string> indexVarNameList = {"i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems a bit long. Whenever possible, lines should be kept to 80 characters max (here and in other parts of code base).

std::vector<IndexVar> getUniqueIndices(size_t order);

public:
LinalgBase(std::string name, Type tensorType, Datatype dtype, std::vector<int> dims, Format format, bool isColVec = false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the constructor need to take both tensorType and dims as arguments? Doesn't tensorType have a shape member that should always be the same as dims?

namespace taco {


struct LinalgVarNode : public LinalgExprNode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and next few classes probably don't need to be indented?


class LinalgBase;

class LinalgRewriter : public util::Uncopyable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name "rewriter" is somewhat confusing since it suggests that rewrite returns a new LinalgStmt or LinalgExpr as input (similar to how IndexNotationRewriter takes IndexExpr as input and returns IndexExpr as output). Maybe something like LinalgLowerer would be less confusing?

return result;
}

IndexExpr LinalgBase::rewrite(LinalgExpr linalg, vector<IndexVar> indices) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably also consider renaming these methods? (See comment regarding LinalgRewriter)

@weiya711 weiya711 self-assigned this May 18, 2021
@rohany rohany moved this from Todo to Doing in Stanford TACO Issues Tracker Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants