NFC - minor spelling tweaks under lite directory by kiszk · Pull Request #35286 · tensorflow/tensorflow

kiszk · 2019-12-19T22:07:11Z

This PR addresses minor spelling tweaks under tensorflow/lite directory.
follow-on of #34958

kiszk · 2019-12-20T02:06:56Z

renjie-liu · 2019-12-20T04:45:52Z

    In particular, if you have 4 inputs to a hint stub, this will be the
    node that you can use as an output. I.e. you have 4 timesteps from a
-    static rnn, then a fused UnidriecitonalLSTM will expect 1 input with
+    static rnn, then a fused UndirecitonalLSTM will expect 1 input with


UnidirectiontionalLSTM

renjie-liu · 2019-12-20T04:48:21Z

  std::string some_name = "something";
  // Don't test float in this case, because precision is hard to predict and
-  // match against, and we don't want a flakey test.
+  // match against, and we don't want a franky test.


renjie-liu

thanks a lot for the fix!

kiszk · 2019-12-20T05:24:00Z

@renjie-liu Thank you for pointing them out. I addressed both of them.

renjie-liu · 2019-12-21T01:48:07Z

 namespace {

-std::string GetMaxUnoolingKernelCode(
+std::string GetMaxUnroolingKernelCode(


should this be unpooling?

renjie-liu · 2019-12-21T01:48:32Z

      if (depth != 1) {
        return InvalidArgumentError(absl::StrCat(
-            "SINGLE_TEXTURE_2D support only cnannels in range [1-4], but ",
+            "SINGLE_TEXTURE_2D support only chnannels in range [1-4], but ",


renjie-liu · 2019-12-21T01:54:49Z

    In particular, if you have 4 inputs to a hint stub, this will be the
    node that you can use as an output. I.e. you have 4 timesteps from a
-    static rnn, then a fused UnidriecitonalLSTM will expect 1 input with
+    static rnn, then a fused UndirectionalLSTM will expect 1 input with


this does not seem to be changed?

this should be UnidirectionalLSTM

You are right. sorry for my overlooking.

renjie-liu

thanks, looks good! please resolve the inline comments

kiszk · 2019-12-21T19:10:30Z

@renjie-liu Thank you. Addressed your three comments.

renjie-liu · 2019-12-22T03:36:23Z


 Status MaxUnpooling::Compile(const CreationContext& creation_context) {
-  const auto code = GetMaxUnoolingKernelCode(
+  const auto code = GetMaxUnroolingKernelCode(


this should be changed as well?

Thank you again, done

gbaned · 2019-12-23T06:28:10Z

@kiszk Can you please address Ubuntu Sanity errors? Thanks!

kiszk · 2019-12-23T08:05:01Z

Thank you for pinging me. I overlooked pylint error. I have just pushed the fixes.

kiszk · 2020-02-29T07:17:57Z

@gbaned @mihaimaruseac Resolved a conflict again.

mihaimaruseac

Probably it would be better to split this even more given that it keeps getting into conflicts.

Let's give it one more try but if we get new conflicts let's try splitting on the next directory level.

mihaimaruseac · 2020-03-02T17:04:57Z

Manually imported the change and synced to head again. If this still fails to merge I'll suggest splitting it as per previous comment

mihaimaruseac · 2020-03-02T20:25:20Z

Turns out this fails to merge properly. I'm starting a new run to eliminate transient errors but I think it would be better to split it (and resync on master). Apologies for the extra work you'll have to do.

mihaimaruseac

Should be split based on subdirectories of tensorflow/lite

gbaned · 2020-03-05T09:38:56Z

@kiszk Can you please check mihaimaruseac's comments and keep us posted? Thanks!

kiszk · 2020-03-05T09:55:33Z

@gbaned thank you for pinging me. I overlooked the comments. I will split this PR into multiple PRs within next few days

tensorflow/lite/
tensorflow/lite/c
tensorflow/lite/delegates
tensorflow/lite/experimental
tensorflow/lite/g3doc
tensorflow/lite/kernels
tensorflow/lite/lib_package
tensorflow/lite/micro
tensorflow/lite/python
tensorflow/lite/testing
tensorflow/lite/toco
tensorflow/lite/tools

gbaned · 2020-03-05T11:07:23Z

@kiszk Sure, Thank you very much for the update.

mihaimaruseac · 2020-03-05T17:06:51Z

You can ping me/assign to me all of the subsequent PRs. Thank you

mihaimaruseac · 2020-03-13T16:46:56Z

Once all subdirectories are fixed, we can sync this back on master to get the files that are left out. Or, we can just close it now and get the other PRs as needed.

Thank you for all the fixes.

kiszk · 2020-03-18T17:36:31Z

I created all of sub-PRs. When they are closed, I think that it would be good to close this PR, too.

mihaimaruseac · 2020-03-18T17:40:44Z

Sounds good. Thank you

mihaimaruseac · 2020-03-21T00:56:49Z

Everything seems solved. Let's rebase this on master if there is still work left to do or close otherwise.

Thank you

kiszk · 2020-03-22T15:29:05Z

@mihaimaruseac Thank you very much. It is the time to close this.

mihaimaruseac · 2020-03-22T15:51:05Z

Thank you for all the work and for the patience to carry on these PRs over 4 months

tensorflow-bot Bot added the size:L CL Change Size: Large label Dec 19, 2019

googlebot added the cla: yes label Dec 19, 2019

kiszk mentioned this pull request Dec 19, 2019

NFC - minor spelling tweaks under python directory #34985

Merged

gbaned self-assigned this Dec 20, 2019

gbaned added the comp:lite TF Lite related issues label Dec 20, 2019

gbaned requested a review from renjie-liu December 20, 2019 04:10

renjie-liu reviewed Dec 20, 2019

View reviewed changes

renjie-liu reviewed Dec 21, 2019

View reviewed changes

renjie-liu previously approved these changes Dec 22, 2019

View reviewed changes

tensorflow-bot Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 22, 2019

kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 22, 2019

kiszk dismissed renjie-liu’s stale review via f24f833 December 22, 2019 04:45

renjie-liu previously approved these changes Dec 23, 2019

View reviewed changes

tensorflow-bot Bot added the kokoro:force-run Tests on submitted change label Dec 23, 2019

kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 23, 2019

gbaned added the kokoro:force-run Tests on submitted change label Dec 23, 2019

kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 23, 2019

gbaned added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Dec 23, 2019

kiszk added 2 commits February 29, 2020 16:16

address review comment

c2919bd

address pylint errors

72e5f5e

mihaimaruseac approved these changes Feb 29, 2020

View reviewed changes

mihaimaruseac suggested changes Mar 3, 2020

View reviewed changes

Conversation

kiszk commented Dec 19, 2019

Uh oh!

kiszk commented Dec 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renjie-liu left a comment

Choose a reason for hiding this comment

Uh oh!

kiszk commented Dec 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renjie-liu left a comment

Choose a reason for hiding this comment

Uh oh!

kiszk commented Dec 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbaned commented Dec 23, 2019

Uh oh!

kiszk commented Dec 23, 2019

Uh oh!

kiszk commented Feb 29, 2020

Uh oh!

mihaimaruseac left a comment

Choose a reason for hiding this comment

Uh oh!

mihaimaruseac commented Mar 2, 2020

Uh oh!

mihaimaruseac commented Mar 2, 2020

Uh oh!

mihaimaruseac left a comment

Choose a reason for hiding this comment

Uh oh!

gbaned commented Mar 5, 2020

Uh oh!

kiszk commented Mar 5, 2020

Uh oh!

gbaned commented Mar 5, 2020

Uh oh!

mihaimaruseac commented Mar 5, 2020

Uh oh!

mihaimaruseac commented Mar 13, 2020

Uh oh!

kiszk commented Mar 18, 2020

Uh oh!

mihaimaruseac commented Mar 18, 2020

Uh oh!

mihaimaruseac commented Mar 21, 2020

Uh oh!

kiszk commented Mar 22, 2020

Uh oh!

mihaimaruseac commented Mar 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants