Skip to content

fixed invalid delete of outputs after execution error in TF #852

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

Merged
merged 3 commits into from
Oct 13, 2021

Conversation

DvirDukhan
Copy link
Collaborator

When a model fails to execute on TF, the excution returns without outputs. On clean it causes to crash

Copy link
Collaborator

@alonre24 alonre24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for it?

env.assertEqual(ret, b'OK')
con.execute_command('AI.TENSORSET', 'my_str_tensor{1}', 'STRING', 4, 'BLOB', "how do I extract keys from a dict into a list?\x00debug public static void main(string[] args) {...}\x00should I use def main()\x00type hinting for list?\x00")
env.assertEqual(ret, b'OK')
check_error(env, con, 'AI.MODELEXECUTE', 'm{1}', 'INPUTS', 1, 'my_str_tensor{1}', 'OUTPUTS', 1, 'foo{1}')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document what is the expected error?

@codecov
Copy link

codecov bot commented Oct 13, 2021

Codecov Report

Merging #852 (f3e815e) into master (de0f302) will increase coverage by 1.35%.
The diff coverage is 91.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #852      +/-   ##
==========================================
+ Coverage   79.97%   81.33%   +1.35%     
==========================================
  Files          53       56       +3     
  Lines        8009     8185     +176     
==========================================
+ Hits         6405     6657     +252     
+ Misses       1604     1528      -76     
Impacted Files Coverage Δ
src/serialization/AOF/rai_aof_rewrite.c 0.00% <0.00%> (ø)
.../serialization/RDB/decoder/previous/v2/decode_v2.c 46.56% <0.00%> (+9.81%) ⬆️
src/execution/parsing/deprecated.c 81.18% <66.66%> (+1.48%) ⬆️
...c/serialization/RDB/decoder/current/v4/decode_v4.c 71.42% <71.42%> (ø)
.../serialization/RDB/decoder/previous/v0/decode_v0.c 63.33% <72.72%> (+1.69%) ⬆️
tests/module/LLAPI.c 75.91% <79.06%> (+1.44%) ⬆️
src/serialization/RDB/encoder/v4/encode_v4.c 85.96% <81.81%> (ø)
src/config/config.c 62.60% <90.90%> (+2.78%) ⬆️
src/backends/tensorflow.c 72.37% <91.26%> (+3.33%) ⬆️
src/backends/onnxruntime.c 77.97% <92.22%> (+0.58%) ⬆️
... and 23 more

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 cdab22e...f3e815e. Read the comment docs.

@DvirDukhan DvirDukhan merged commit e4e5d98 into master Oct 13, 2021
@DvirDukhan DvirDukhan deleted the fix_tf_invalid_output_delete branch October 13, 2021 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants