support 64-bit Int in WDL (using Long internally in WomInteger) #2685

[natechols]

First attempt, works end-to-end for my use case.

[natechols]

As usual I can't really tell what the CI failures mean... they don't seem correlated to anything in particular?

[aednichols]

Merge develop and re-push, you may be unlucky or your branch may be missing test updates

[aednichols]

[error] (wdlModelDraft2 / Test / compileIncremental) Compilation failed

https://travis-ci.com/github/broadinstitute/cromwell/jobs/473221961

[natechols]

I'm confused, I fixed the compile failure and merged with develop again and now there are more build failures - which do not appear to be consistent with each other?

[natechols]

@aednichols Is there anything else I need to do for this or is it waiting in the review queue?

[aednichols]

You're good; your code looks right but I need to check with the team that this is the right thing to do. It would be bad if we subtly broke a few peoples' workflows.

[natechols]

ping

[natechols]

@aednichols @cjllanwarne Any updates?

[aednichols]

Still queued internally.

[cjllanwarne]

hi @natechols, one comment I have is that there is already a WOM type for 64-bit integers (called WomLong). Right now that's only generated by CWL workflows requesting longs, but perhaps a better way to achieve the desired result in WDL would be to create WomLongs from the WDL parsers instead of the current WomIntegers they generate.

I believe if we did things the way this PR is currently implemented that it would be impossible to have an int type in CWL anymore (per https://www.commonwl.org/v1.0/CommandLineTool.html#CWLType) because there would be no 32-bit datatype left in WOM

[natechols]

@cjllanwarne I considered doing this at first and I think I actually tried to implement it, but there was a fundamental inconsistency that I couldn't figure out how to resolve. It might have had to do with going from the WomLong type back to a WDL type, but I hit so many errors along the way that I may be mixing them up. The current implementation is what I eventually settled on as the least invasive (not necessarily the most elegant). If the current approach is a deal-breaker I can take another look, but I suspect the blast radius will be larger no matter what. My hope was that CWL users wouldn't actually notice or care about the underlying JVM type - is there a use case for large integer arrays in Cromwell?

[natechols]

Is this just a WONTFIX? I'm getting complaints from coworkers about this bug again.

[aednichols]

I think it's more like "won't merge" (this PR as it stands) rather than "won't fix".

[natechols]

@aednichols In that case is there an alternative implementation path that would be acceptable? I am not wedded to this way of doing it, but I've heard zero feedback on what to do instead and I'm reluctant to start coding anything new until I know whether it will actually be reviewed.