miniwdl icon indicating copy to clipboard operation
miniwdl copied to clipboard

Miniwdl does not allow size on an array.

Open rhpvorderman opened this issue 4 years ago • 5 comments

Dear mlin,

I was testing the testathon-2020 test suite with miniwdl. I noticed a bug in tests/stdlib/size/test_size.wdl.

Miniwdl version 0.6.4 crashes with the following messages:

2020-02-17 10:59:17,106 wdl.w:test_size NOTICE miniwdl :: version: "v0.6.4"
2020-02-17 10:59:17,106 wdl.w:test_size NOTICE workflow start :: name: "test_size", source: "tests/stdlib/size/test_size.wdl", line: 3, column: 1, dir: "/home/rhpvorderman/PycharmProjects/Testathon-2020/20200217_105917_test_size"
2020-02-17 10:59:17,107 wdl.w:test_size NOTICE workflow steps :: waiting: 12, outstanding: 0, finished: 0
2020-02-17 10:59:17,116 wdl.w:test_size ERROR workflow test_size (tests/stdlib/size/test_size.wdl Ln 3 Col 1) failed :: error: "NullValue", message: "Null value", node: "output-size_all"
2020-02-17 10:59:17,116 wdl.w:test_size NOTICE aborting workflow
2020-02-17 10:59:17,117 miniwdl-run ERROR workflow test_size (tests/stdlib/size/test_size.wdl Ln 3 Col 1) failed :: dir: "/home/rhpvorderman/PycharmProjects/Testathon-2020/20200217_105917_test_size"
2020-02-17 10:59:17,117 miniwdl-run ERROR (tests/stdlib/size/test_size.wdl Ln 30 Col 32) NullValue, Null value

Line 30 in the wdl file looks like this:

        Float size_all = size([f0, f1, f2])

Hope this helps. If you need any more information feel free to ask.

Best regards, rhpvorderman

rhpvorderman avatar Feb 17 '20 10:02 rhpvorderman

@mlin. Ah I see you also ran into this. Was it solved by this commit by any chance?

I noticed the issue when working on this PR for the testathlon.

rhpvorderman avatar Feb 17 '20 10:02 rhpvorderman

@rhpvorderman Yes exactly! Even after fixing that, I discovered that Cromwell uses e.g. 2^20 for both "MB" and "MiB", whereas miniwdl uses 10^6 and 2^20 respectively. This remains to be resolved. So that has already turned out to be a very fruitful portability test case, LOL!

mlin avatar Feb 17 '20 19:02 mlin

miniwdl v0.6.5 includes the patch so that size() doesn't crash given a null input, but the unit definitions remain an outstanding discrepancy with Cromwell.

mlin avatar Feb 18 '20 01:02 mlin

@mlin. Indeed the test case does not crash on size now but on:

2020-02-18 08:34:42,782 wdl.w:test_size NOTICE miniwdl :: version: "v0.6.5"
2020-02-18 08:34:42,782 wdl.w:test_size NOTICE workflow start :: name: "test_size", source: "tests/stdlib/size/test_size.wdl", line: 3, column: 1, dir: "/tmp/pytest_workflow_cwrhn89_/size_test_miniwdl/20200218_083442_test_size"
2020-02-18 08:34:42,783 wdl.w:test_size NOTICE workflow steps :: waiting: 12, outstanding: 0, finished: 0
2020-02-18 08:34:42,789 wdl.w:test_size ERROR workflow test_size (tests/stdlib/size/test_size.wdl Ln 3 Col 1) failed :: error: "EvalError", message: "size(): invalid unit B", node: "decl-sz_u0-00"
2020-02-18 08:34:42,789 wdl.w:test_size NOTICE aborting workflow
2020-02-18 08:34:42,789 miniwdl-run ERROR workflow test_size (tests/stdlib/size/test_size.wdl Ln 3 Col 1) failed :: dir: "/tmp/pytest_workflow_cwrhn89_/size_test_miniwdl/20200218_083442_test_size"
2020-02-18 08:34:42,789 miniwdl-run ERROR (tests/stdlib/size/test_size.wdl Ln 22 Col 23) EvalError, size(): invalid unit B

invalid unit B.

I looked in the spec and it says

Supported units are KiloByte ("K", "KB"), MegaByte ("M", "MB"), GigaByte ("G", "GB"), TeraByte ("T", "TB") as well as their binary version "Ki" ("KiB"), "Mi" ("MiB"), "Gi" ("GiB"), "Ti" ("TiB"). Default unit is Bytes ("B").

So the default unit B is not supported according to the spec :wink:. MiniWDL is compliant!

rhpvorderman avatar Feb 18 '20 07:02 rhpvorderman

LOL

mlin avatar Feb 18 '20 08:02 mlin