updates for CWL CUDA, Resources, NetworkAccess requirements

[fmigneault]

Changes

• Update documentation with examples for cwltool:CUDARequirement, ResourceRequirement and NetworkAccess.
• Improve schema definition of ResourceRequirement.
• Deprecate DockerGpuRequirement, with attempts to auto-convert it into corresponding DockerRequirement
  combined with cwltool:CUDARequirement definitions. If this conversion does not work transparently for the user,
  explicit CWL updates with those definitions should be made.
• Ensure that validation check finds exactly one provided CWL requirement or hint to represent the application type.
  In case of missing requirement, the reported error will contain a documentation link to guide the user in adjusting
  its Application Package accordingly.

Refernces

• Temporary documentation link used due to No reference documentation for cwltool:CUDARequirement common-workflow-language/user_guide#414
• Relates to $schema and $schemas common-workflow-language/cwl-v1.2#138
• Fixes https://crim-ca.atlassian.net/browse/DAC-437

TODO

• tests for new _update_package_compatibility operation
• tests for result ResourceRequirement schema validation
• documentation of CWL-like hint/requirement for remote process

[codecov]

Codecov Report

Merging #506 (96c49c0) into master (1a92e1c) will increase coverage by 0.08%.
The diff coverage is 97.50%.

@@            Coverage Diff             @@
##           master     #506      +/-   ##
==========================================
+ Coverage   83.74%   83.82%   +0.08%     
==========================================
  Files          78       78              
  Lines       16256    16349      +93     
  Branches     2509     2525      +16     
==========================================
+ Hits        13614    13705      +91     
- Misses       1945     1947       +2     
  Partials      697      697              

Impacted Files	Coverage Δ
weaver/processes/convert.py	83.41% <ø> (ø)
weaver/store/mongodb.py	76.39% <ø> (ø)
weaver/processes/wps_package.py	82.50% <94.33%> (+0.50%)	⬆️
weaver/datatype.py	75.91% <100.00%> (+0.17%)	⬆️
weaver/exceptions.py	92.03% <100.00%> (+0.07%)	⬆️
weaver/processes/builtin/__init__.py	88.79% <100.00%> (ø)
weaver/processes/constants.py	100.00% <100.00%> (ø)
weaver/wps_restapi/swagger_definitions.py	99.92% <100.00%> (+<0.01%)	⬆️
weaver/processes/utils.py	80.58% <0.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ChaamC]

Very solid PR, good job!
Minor question to verify about the cuda requirement.

[ChaamC]

About using the Cuda requirements in a requirements section, I am not sure it is working.
When I tested using them in the requirements, I would get an error during process deployment.

  <h1>422 Unprocessable Entity</h1>
  Unable to process the contained instructions<br/><br/>
Invalid package/reference definition. Loading generated error: [Invalid package/reference definition. Loading package content generated error: [../../../tmp/tmpa7ns6fvb/package:1:52:  checking field `requirements`
../../../tmp/tmpa7ns6fvb/package:1:172:   checking item
                                            Field `class` contains undefined reference to
                                            `http://commonwl.org/cwltool#CUDARequirement`].

I only used hints for it to work. Consequently, the CUDARequirementSpecification was only added to hints in the swagger_definitions.py.
So I am not sure if these examples are relevant since Cuda was not working as a requirements.
Sorry if that was not clear in my previous PR. I suppose this might be fixed eventually if they update the doc on their side? Those examples would be the desired behavior if the requirements would work, so maybe we can keep them and leave a comment about it? What do you think would be best here? Are you able to recreate the error I get?

[fmigneault]

@mr-c
Can you confirm this case? The cwltool:CUDARequirement is only permitted in hints?
If so, that's another thing that could be specified in https://github.com/common-workflow-language/cwl-v1.2/issues/212

[mr-c]

cwltool:CUDARequirement should almost always be under requirements unless it truly is optional. When using cwltool one needs to pass --enable-ext. See https://github.com/common-workflow-language/cwltool/blob/0099f11d5a8740d3ca976cd40681523d1002e502/cwltool/main.py#LL661C15-L661C24
You could choose to permanently enable some of the extensions in weaver; it is up to you if you want a command-line flag or not.

[fmigneault]

Is there an equivalent of --enable-ext directly with the cwltool code rather than the CLI?

We use the python factory with loading/runtime context and following parameters:
https://github.com/crim-ca/weaver/blob/master/weaver/processes/wps_package.py#L1288-L1309

[mr-c]

Currently setup_schema (which handles --enable-ext) is only called from the CLI loop. So you'll need to call use_custom_schema() at startup before using any of the factory methods.

If you don't want to enable all extensions, then make your own copy of extensions{,-v1.1,-v1.2}.yml; otherwise you can use the copy inside the cwltool module.

If you'd like, we can refactor setup_schema to take in a boolean named "enable_ext" instead of an entire argparse.Namespace

[fmigneault]

@mr-c
I have tried using use_custom_schema (#510) as suggested, but I end up having the following error when the CWL document is loaded by the factory.

schema_salad.exceptions.ValidationException: http://commonwl.org/cwltool:2:3:  checking object `http://commonwl.org/cwltool#CUDARequirement`
http://commonwl.org/cwltool:5:3:    Field `extends` references unknown identifier
                                    `cwl:ProcessRequirement`, tried
                                    http://commonwl.org/cwltool#cwl:ProcessRequirement
http://commonwl.org/cwltool:6:3:    checking field `fields`
http://commonwl.org/cwltool:22:5:     checking object
                                      `http://commonwl.org/cwltool#CUDARequirement/cudaDeviceCountMax`
                                        Field `type` references unknown identifier
                                        `cwl:Expression`, tried http://commonwl.org/cwltool#cwl:Expression
http://commonwl.org/cwltool:32:5:     checking object
                                      `http://commonwl.org/cwltool#CUDARequirement/cudaDeviceCountMin`
                                        Field `type` references unknown identifier
                                        `cwl:Expression`, tried http://commonwl.org/cwltool#cwl:Expression

Instead of the previous (expected) error:

schema_salad.exceptions.ValidationException: ../../../../tmp/tmp514w9uk1/test:1:114: checking field `requirements`
../../../../tmp/tmp514w9uk1/test:1:131:   checking item
                                            Field `class` contains undefined reference to
                                            `http://commonwl.org/cwltool#CUDARequirement`

Is there something else to adjust on my end, or would that be an issue with the schema definitions (using https://github.com/common-workflow-language/cwltool/blob/3.1.20221201130942/cwltool/extensions-v1.2.yml)?

Tested CWL:

cwlVersion: v1.2
class: CommandLineTool
baseCommand:
- echo
- test
inputs: []
outputs: []
requirements:
  cwltool:CUDARequirement:
    cudaVersionMin: '10.0'
    cudaComputeCapability: '3.0'
    cudaDeviceCountMin: 1
    cudaDeviceCountMax: 1
$namespaces:
  cwltool: http://commonwl.org/cwltool#

[mr-c]

Perhaps you filtered out too much? https://github.com/common-workflow-language/cwltool/blob/38f42e6c4fe17db5fe82acd8a59f97d3982c605b/cwltool/extensions-v1.2.yml#L1-L5 might be missing due to https://github.com/crim-ca/weaver/pull/510/files#diff-ce41af74805c1d891a393bd1488434a450041ca7cc7df4f3532b4e516badf656R426

Since you are pinning your cwltool version, it might be easier to just make your own copy of https://github.com/common-workflow-language/cwltool/blob/38f42e6c4fe17db5fe82acd8a59f97d3982c605b/cwltool/extensions-v1.2.yml and remove the unsupported extensions

[fmigneault]

The import statement is still in the $graph. The only extensions removed are the ones I marked as not yet supported.

[mr-c]

Can you serialize the newly produced extension file and send me a link?

[fmigneault]

I just found my mistake. I was passing down $graph rather than the full definition to use_custom_schema. Works as expected now!

[mr-c]

• Deprecate DockerGpuRequirement, with attempts to auto-convert it into corresponding DockerRequirement
  combined with cwltool:CUDARequirement definitions. If this conversion does not work transparently for the user,
  explicit CWL updates with those definitions should be made.

I'm glad to see this and I'll eventually add similar code to cwl-upgrader when CUDARequirement is part of a released version of CWL (likely v1.3).

What is the namespace for the DockerGpuRequirement extension?

[fmigneault]

@mr-c

What is the namespace for the DockerGpuRequirement extension?

There was no specific namespace employed. It was only used in hints and warnings were ignored.
Unless someone copied the idea, it should only exist in Weaver.

[fmigneault]

Merging as is for the moment.
The issue discussed in #506 (review) (allow CUDARequirement in requirements section) will be handled by subsequent PR #510.