lf1-io / padl

Functional deep learning
Apache License 2.0
106 stars 4 forks source link

Remove first marker #405

Closed eramdiaz closed 2 years ago

eramdiaz commented 2 years ago

Description

Fixes #404 and #340 by not showing anymore the arrow at the highest level of the debugger (where the entire transform we are executing is shown).

For example, let t be

t = (transform(lambda x: torch.LongTensor(x))
     >> batch
     >> transform(torch.nn.Embedding)(10, 8)
     >> transform(torch.nn.Linear)(6, 4)
)

Then, an error will be produced at the linear layer due to mismatching shapes. If we execute and call the debugger, at the first level we will get

Compose - "t":

      │
      ▼ x
   0: lambda x: torch.LongTensor(x)                
      │
      ▼ args
   1: Batchify(dim=0)                              
      │
      ▼ input
   2: Embedding(num_embeddings=10, embedding_dim=8)
      │
      ▼ input
   3: Linear(in_features=6, out_features=4)      

as we see with no marker arrow. The deeper levels will already provide the marker arrow. For example, the next one is


Compose:

      │
      ▼ input
   0: Embedding(num_embeddings=10, embedding_dim=8)
      │
      ▼ input
   1: Linear(in_features=6, out_features=4)          <---- error here 
codecov-commenter commented 2 years ago

Codecov Report

Merging #405 (6a571d7) into main (5aedf81) will increase coverage by 0.25%. The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #405      +/-   ##
==========================================
+ Coverage   87.35%   87.60%   +0.25%     
==========================================
  Files          15       15              
  Lines        3012     2978      -34     
==========================================
- Hits         2631     2609      -22     
+ Misses        381      369      -12     
Impacted Files Coverage Δ
padl/transforms.py 91.60% <20.00%> (+0.69%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5aedf81...6a571d7. Read the comment docs.

wuhu commented 2 years ago

Why did you decide to remove the top level marker?

eramdiaz commented 2 years ago

This is supposed to solve the issue #404 (and #340), which describes a bug on the top level marker in a non edgy case. It is not a direct fix but I don't know how to solve it in a direct way. Let me know if you agree with the fix @wuhu :)

wuhu commented 2 years ago

@eramdiaz I got that, I was trying to understand why you want to remove the marker entirely on the top level, rather than making it work in those cases - Is it not possible to fix it in a direct way or do you think it's too complicated / not worth the effort?