Mapping of pyttb ttb functionality #291

jeremy-myers · 2023-12-12T19:13:33Z

Addresses #180

Summary: this PR came from a desire to ease the transition for MATLAB TTB users to pyttb. The main goal of mapping TTB and pyttb functionality is to document where naming conventions, method and data member usages, etc. differ between implementations.

📚 Documentation preview 📚: https://pyttb--291.org.readthedocs.build/en/291/

…nstruction-of-a-tucker-tensor.

…b and pyttb documentation, 2. add algorithms.

jeremy-myers · 2023-12-12T19:22:51Z

This PR fails regression tests, but only documentation was updated (no source code).

ntjohnson1 · 2023-12-13T18:21:11Z

It looks like there was a breaking change in ruff. Could try pip install --upgrade ruff which should repro locally. Then the failure says it an auto fixable change so ruff check --fix . from root should resolve. We probably should pin our ruff version.

jeremy-myers · 2023-12-13T18:41:16Z

My pre-commit failed to update ruff due to proxy issues and missed this error. I'm not sure how to resolve these extra args calls in cp_apr and fg_est.

ntjohnson1 · 2023-12-13T18:47:21Z

This is a general configuration issue. Unrelated to your change like you mentioned. It looks like pre-commit requires (or I just set it up this way), a pinned version of ruff here that hasn't conflicted with latest ruff until now. The simplest unblocking solution is probably to update our package ruff requirement here to match exactly. This will require rolling back your latest commit and downgrading ruff locally. That pinned version is why pre-commit passed last time but the pytest failed (I assumed it was just caching but clearing the cache wasn't successful).

Then can file a ticket to handle this cleaner. Ideally linking the two configs and not pinning to an EXACT ruff version.

ntjohnson1 · 2023-12-13T18:48:25Z

If that doesn't work I can debug it after business hours today on a separate branch.

jeremy-myers · 2023-12-13T19:21:52Z

@ntjohnson1 It sounds like I should wait for the pre-commit yaml and/or pyproject to be updated to unblock?

ntjohnson1 · 2023-12-13T19:47:22Z

Confirmed this unblocks and I think it is a reasonable temporary or even medium term solution since its only a dev requirement.

IMO I think the easiest approach is to cherry-pick the commit from there or just apply the change yourself and revert 75c0f1b. No need for a separate PR to unblock.

…om/jeremy-myers/pyttb into mapping-of-pyttb-ttb-functionality

jeremy-myers · 2024-02-05T21:14:42Z

@ntjohnson1 It looks like this fix is now failing with black --check .. Is this another pinning issue?

ntjohnson1 · 2024-02-05T21:19:24Z

Boo. I assume so since it looks like we have it pinned here

pyttb/.pre-commit-config.yaml

Line 8 in 5751f02

rev: 23.3.0

. Should be quick to test if it fixes the issue.

…mmit hooks.

ntjohnson1 · 2024-02-05T21:58:48Z

pyproject.toml

@@ -36,7 +36,7 @@ dev = [
 "nbstripout",
 "pytest",
 "pytest-cov",
- "ruff",
+ "ruff==0.0.284",


You updated the pre-commit config but left this pinned. So we still have the issue of incompatible pre-commit and project. My suggestion is to be more explicit in our pyproject so the alignment is guaranteed then figure out an easier way to keep them aligned in the future as a follow up

I think I have it. I pinned ruff but pre-commit autoupdate gave a conflicting version. So, pre-commit autoupdate --repo https://github.com/psf/black to update black only should do the trick.

The tests pass now but we still have the possibility of this pre-commit/black mismatch occurring again. Can you pin the version of black in this file to match the one in the pre-commit. We can figure out if there is an automated way to align these as follow up. In my opinion it would be nicer to keep the version of black that was originally in in the pre-commit and just pin it here then roll back the changes from the newer version of black. That way the content changes and formatting churned aren't coupled together.

…black only.

…om/jeremy-myers/pyttb into mapping-of-pyttb-ttb-functionality

ntjohnson1

Will leave for Danny to give the official approval and merge it in. Thanks for putting this together.

A few notes:

My preference for all the formatting headache would be to pin black in pyproject to the previous version that is on main, rollback the changes in *.ipynb and *.py.
As I mentioned in the tenmat doc some of these things may go stale and there isn't currently a way to check if the pyttb side is broken. Could you file a follow up ticket to add some sort of automated checking for this content? Either using Jupyter notebooks and the rendering we already have or sphinx supports doctest, but I don't think we have an example of that in the repo yet. Wouldn't need to verify outputs but just a smoke test basically to make sure the methods/functions actually exist.

ntjohnson1 · 2024-02-07T11:05:40Z

pyproject.toml

@@ -36,7 +36,7 @@ dev = [
 "nbstripout",
 "pytest",
 "pytest-cov",
- "ruff",
+ "ruff==0.0.284",


The tests pass now but we still have the possibility of this pre-commit/black mismatch occurring again. Can you pin the version of black in this file to match the one in the pre-commit. We can figure out if there is an automated way to align these as follow up. In my opinion it would be nicer to keep the version of black that was originally in in the pre-commit and just pin it here then roll back the changes from the newer version of black. That way the content changes and formatting churned aren't coupled together.

ntjohnson1 · 2024-02-07T11:11:07Z

docs/source/matlab/common.rst

+=================+======================+========================================================================+
+| ``and`` | ``logical_and`` | ``X.logical_and(Y)`` |
+-----------------+----------------------+------------------------------------------------------------------------+
+| ``disp`` | ``__str__`` | ``X`` |


Hmm, for all the names pyttb names that include dunder methods I wonder if it makes sense to use their public equivalents. Technically they are all hidden methods that get exposed via some other hook. For example __str__ and __repr__ both get called under the hood for str(X) and repr(X). For someone totally unaware of python just trying to directly translate from matlab based on this doc I'd really hope they don't end up with a bunch of __str__, __rmul__ calls in their code

ntjohnson1 · 2024-02-07T11:12:55Z

docs/source/matlab/common.rst

+| ``xor`` | ``logical_xor`` | ``X.logical_xor(Y)`` |
+-----------------+----------------------+------------------------------------------------------------------------+
+
+Copying


It might be useful to note this is fairly common for non-builtin types in python and include a link. Hereish? https://docs.python.org/3/library/copy.html

ntjohnson1 · 2024-02-07T11:14:50Z

docs/source/matlab/symktensor.rst

@@ -0,0 +1,8 @@
+``symktensor``


IMO I think it might be clearer to just remove the placeholders. I'm not sure what the plan is for actually covering these other classes.

ntjohnson1 · 2024-02-07T11:16:39Z

docs/source/matlab/tenmat.rst

@@ -0,0 +1,22 @@
+``tenmat``


I have a PR up now that updates the tenmat constructors. I believe this is correct currently but once Danny merges my PR this will go stale.

ntjohnson1 · 2024-02-07T11:21:49Z

docs/source/for_matlab_users.rst

+======================
+
+Already familiar with MATLAB Tensor Toolbox? To assist transitions from the
+Tensor Toolbox for MATLAB to pyttb, this guide documents some key differences.


It might be good to mention that the tutorials are similar to the MATLAB ones if they want a hands on approach. I think that's already implied but might not be a bad idea to be explicit.

jeremy-myers and others added 17 commits September 19, 2023 14:26

Created ttensor/reconstruct tutorial in master.

feefeb9

Merge with upstream

afd927f

autoformat

5b3f607

Removed from main until completed in branch 248-tutorial-partial-reco…

05a37a1

…nstruction-of-a-tucker-tensor.

Merge with upstream

b115908

Merge with upstream

2dceaff

Merge with upstream.

3311b11

Merge branch 'main' of https://github.com/sandialabs/pyttb

982a6cd

Initial commit

b310115

Basic matlab to pyttb mapping. TODOs: 1. insert hyperlinks to both tt…

38ec12b

…b and pyttb documentation, 2. add algorithms.

Merge branch 'main' of https://github.com/jeremy-myers/pyttb

f3babc9

nbstripout

ccef927

Merge branch 'sandialabs:main' into main

19680f2

Merge branch 'sandialabs:main' into main

8d1c32f

Merge branch 'main' into mapping-of-pyttb-ttb-functionality

af30fe5

Merge branch 'sandialabs:main' into mapping-of-pyttb-ttb-functionality

16a9f01

Initial version ready.

a7b595f

jeremy-myers requested a review from dmdunla December 12, 2023 19:22

Updates to ruff

75c0f1b

jeremy-myers added 4 commits February 5, 2024 10:05

Updates to ruff

ea3613e

Merge branch 'mapping-of-pyttb-ttb-functionality' of https://github.c…

ebd328a

…om/jeremy-myers/pyttb into mapping-of-pyttb-ttb-functionality

Ruff fixes.

7b71366

Ruff fixes.

960fac8

jeremy-myers added 2 commits February 5, 2024 13:36

Attempting pre-commit fixes with autoupdate.

3d0e814

Attempting pre-commit fixes with autoupdate: part two, running pre-co…

a65d7cd

…mmit hooks.

ntjohnson1 reviewed Feb 5, 2024

View reviewed changes

jeremy-myers added 5 commits February 5, 2024 13:58

Attempting pre-commit fixes with autoupdate: attempt two, autoupdate …

6d300c1

…black only.

Merge branch 'mapping-of-pyttb-ttb-functionality' of https://github.c…

9379112

…om/jeremy-myers/pyttb into mapping-of-pyttb-ttb-functionality

Attempting to fix pre-commit issues and noqas

c42b3ab

Pin ruff, fix precommit

4c35c29

Run precommit

a9fe513

jeremy-myers requested a review from ntjohnson1 February 5, 2024 23:58

ntjohnson1 reviewed Feb 7, 2024

View reviewed changes

dmdunla approved these changes Apr 3, 2024

View reviewed changes

dmdunla merged commit 42f1289 into sandialabs:main Apr 3, 2024
8 checks passed

jeremy-myers deleted the mapping-of-pyttb-ttb-functionality branch April 3, 2024 20:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mapping of pyttb ttb functionality #291

Mapping of pyttb ttb functionality #291

jeremy-myers commented Dec 12, 2023 •

edited by github-actions bot

Loading

jeremy-myers commented Dec 12, 2023

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Dec 13, 2023

ntjohnson1 commented Dec 13, 2023 •

edited

Loading

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Dec 13, 2023

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Feb 5, 2024

ntjohnson1 commented Feb 5, 2024

ntjohnson1 Feb 5, 2024 •

edited

Loading

jeremy-myers Feb 5, 2024

ntjohnson1 Feb 7, 2024

ntjohnson1 left a comment

ntjohnson1 Feb 7, 2024

ntjohnson1 Feb 7, 2024

ntjohnson1 Feb 7, 2024

ntjohnson1 Feb 7, 2024

ntjohnson1 Feb 7, 2024

ntjohnson1 Feb 7, 2024

Mapping of pyttb ttb functionality #291

Mapping of pyttb ttb functionality #291

Conversation

jeremy-myers commented Dec 12, 2023 • edited by github-actions bot Loading

jeremy-myers commented Dec 12, 2023

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Dec 13, 2023

ntjohnson1 commented Dec 13, 2023 • edited Loading

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Dec 13, 2023

ntjohnson1 commented Dec 13, 2023

jeremy-myers commented Feb 5, 2024

ntjohnson1 commented Feb 5, 2024

ntjohnson1 Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntjohnson1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremy-myers commented Dec 12, 2023 •

edited by github-actions bot

Loading

ntjohnson1 commented Dec 13, 2023 •

edited

Loading

ntjohnson1 Feb 5, 2024 •

edited

Loading