Push explicit Fortran Ordering Through Docs

[ntjohnson1]

With #354 we made our Fortran ordering more explicit. We should capture that directly on our docs and for all our examples/tutorials we should address anywhere we are getting warnings about Fortran orderings (this may catch places we aren't tracking/converting correctly).

[ntjohnson1]

FYI @tgkolda I fixed a few internal calls that resolved the warnings in all our tutorials. I didn't have time to add more notes so I haven't opened a PR for it yet but if the warnings are annoying feel free to check out https://github.com/ntjohnson1/pyttb/tree/nick/fortran_ordering

[tgkolda]

Regarding #371, I know everything is working correctly in terms of giving the right answer, but efficiency is a concern. Why isn't the constructor making the tensor in F-order to begin with? And I don't have the Python debugging to know where the tensor needs to be F-ordered. I'm wondering if we should have a strategy discussion? There aren't that many functions where the ordering is important, and we should be able to handle both F and C orderings equally efficiently.

[ntjohnson1]

Have you had a chance to look at the branch I linked above? These warnings are generated from internal code that actually doesn't depend on how the user is calling things, basically its a bug that I think I squashed.

I know everything is working correctly in terms of giving the right answer, but efficiency is a concern.

IMO correct coverage is a bigger focus at the moment since we are pretty heavily bandwidth constrained. I'm not sure if Danny has more bandwidth or additional people do in the new year.

Why isn't the constructor making the tensor in F-order to begin with?

The constructor does make the tensor F-ordered to begin with. Our constructors have a copy argument, if copy is set to False but the input data is in C-ordering, we must make a copy and that's what raises the error. For a variety of operations numpy will only return a C-ordered array so we have to make a copy to transform to F-ordering. This isn't an issue for the user but rather an issue with some of our internal code. In the linked branch I added tests (so far just to dense tensors) that every operation that returns an array returns it in F-order for consistency.

I'm wondering if we should have a strategy discussion? There aren't that many functions where the ordering is important, and we should be able to handle both F and C orderings equally efficiently.

This might be worthwhile when Danny is back.

Previously, we were only dealing with the ordering in areas where it impacted our final result. All interactions with the tensor classes were consistent but as you pointed out that meant that interacting with a dense tensor vs the underlying numpy array that supported the dense tensor could yield incompatible results (linear index to tensor would be F-order, where a ravel then linear index on the array would be C-order).

Once we factor in chained operations I think it is difficult to say how hard it is to handle both orderings. I assume if we want to handle both orderings we basically want to be explicit and consistent (C-ordered tensors only deal in C-orderings, and F-ordered tensors only deal in F orderings). I've started to propagate that through the code base in the linked branch. Right now only testing F-ordering but parameterizing the order argument and adding utilities to support both approaches. When we prioritize C support we'll probably also have to parameterize our tests/make sure we are correctly comparing results for both layouts.

[tgkolda]

I'm confused by the comment that the constructor makes the tensor F-ordered. If that's the case, why was I getting errors from the code in #371?

[ntjohnson1]

This is the commit where I resolved (at least most).

For illustration (more fixes in the commit) in cp_apr the warning is raised because:

 X_mat = input_tensor.to_tenmat(np.array([n]), copy=False).data

This is internal to the implementation of cp_apr where we create a numpy array based on an integer (but is C-ordered by default) and we set the constructor to not copy to avoid wasting resources. This would raise the warning every time that line is hit in cp_apr. There is no change a user could make to avoid this warning since it is an internal bug (bug that the warning is raised, but the constructor does convert the data to F-ordering).

The fix I made was to create the array in a consistent memory layout.

X_mat = input_tensor.to_tenmat(
   np.array([n], order=input_tensor.order), copy=False
).data

[tgkolda]

I'm not calling cp_apr in the code snippet in #371. Maybe you're using this as an example, but I'm not familiar enough with the cp_apr code to understand the point or how it relates to the code snippet where I encountered problems.

[ntjohnson1]

That was just an exemplar on how that error could be raised internally no matter what the user did. cp_als probably had similar internal operations. In the branch I linked to I resolved the warnings (last I checked). You can try rerunning your snippet with that branch or I can look at it later.

I was trying to highlight the concept of why the warning would raise if the constructor converts things to Fortran order. The warning raises if a user provides C-ordered data AND explicitly states they just want the tensor to reference the original data rather than copy it. By default we make a copy at construction and most users won't set copy to False, but if they do it's good to loudly state we cannot honor that. Alternatively we could just raise.

[tgkolda]

Okay. I'll try to see if I can figure out where that is happening.

[ntjohnson1]

Okay. I'll try to see if I can figure out where that is happening.

Just trying to clarify since we both seem to be a little confused. This is already fixed on the branch associated with this current issue (which is why I closed #371). I just ran the code snippet on the branch to confirm. Mostly I was trying to explain why you were seeing the issue in the first place. I wasn't requesting that you debug it (definitely don't want to debug off main or we're fixing the same thing twice).

[tgkolda]

I see. I switched to your branch and this problem has gone away. Thanks!