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

do einops' operations account for contiguous memory layout? #88

Open
CDitzel opened this issue Nov 16, 2020 · 5 comments
Open

do einops' operations account for contiguous memory layout? #88

CDitzel opened this issue Nov 16, 2020 · 5 comments
Labels
question Further information is requested

Comments

@CDitzel
Copy link

CDitzel commented Nov 16, 2020

Upon heavy reshaping and dimension manipulations, it is necessary from time to time to call .contiguous() on the resulting tensors to straighten out the memory layout. Does einops account for this automatically? I dont see no call to contiguous() anywhere in the examples

@arogozhnikov arogozhnikov added the question Further information is requested label Nov 17, 2020
@arogozhnikov
Copy link
Owner

Good question!

Let's assume we are talking in pytorch terms.
It is common to use .contiguous() before using view in pytorch, however einops replaces view (e.g. with rearrange).

Einops operations can work with any memory layout and internally flatten array if that's required.
However when possible einops returns view to original tensor, not copy (in agreement with how built-in reshape/transpose work).

There is afaik no need to provide contiguous tensors to other operations (besides view), but if for some reason it is required - you can always use .contigous()

@arogozhnikov
Copy link
Owner

I'll keep this question open for reference

@arogozhnikov arogozhnikov reopened this Nov 17, 2020
@CDitzel
Copy link
Author

CDitzel commented Feb 18, 2021

so I recently had the problem that pytorch would throw me a warning lile

[W accumulate_grad.h:170] Warning: grad and param do not obey the gradient layout contract. This is not an error, but may impair performance.
grad.sizes() = [1, 64, 1, 1], strides() = [64, 1, 1, 1]
param.sizes() = [1, 64, 1, 1], strides() = [64, 1, 64, 64] (function operator())

and the line which causes the issueing of this warning was

constituents = rearrange(constituents, "b (h w) d -> b d h w", h=h, w=w)

it was gone after I changed it to

constituents = rearrange(constituents, "b (h w) d -> b d h w", h=h, w=w).contiguous()

so I thougt you wanna know that. Maybe it is an option to call contiguous anywars within einops?

@arogozhnikov
Copy link
Owner

Hi @CDitzel ,

Maybe it is an option to call contiguous anywars within einops?

All operations try to incur minimal overhead in terms of memory and time, returning views when possible is the right policy (e.g. transpose in pytorch/numpy works the same way). Returning contiguous always would result in higher memory consumption even when not necessary.

and the line which causes the issueing of this warning was

Line that causes warning is likely the next operation because that op is not optimized for non-cont layout but does not call .contiguous for the same reasons einops does not this - to avoid overhead.

Let me state - everything works how it should work

  1. pytorch reports potentially slow place
  2. what you've done by adding .contiguous() is the right approach. Downstream code wants contiguous array - you make this explicit additional step to provide it.

You also want to check that speed is improved after this addition. If not - you can skip .contiguous and spend less memory on that.

Warning you observe looks new to me, but it's good news that pytorch starts reporting potentially non-optimal places. Non-informativeness of warning you see is disappointing though.

@CDitzel
Copy link
Author

CDitzel commented Feb 23, 2021

thank you for your reply. I agree that this topic should be kept open for future references

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants