cwltool doesn't complain when max is lesser than min in ResourceRequirements

[Stellatsuu]

Following this discussion: https://cwl.discourse.group/t/how-does-resourcerequirement-work/1019, cwltool does not validate whether resourceMax is greater than resourceMin in ResourceRequirements.

According to the CWL documentation, this should not be allowed, as it states: "It is an error if max < min.".

[aldbr]

(I work with @Stellatsuu) Looks like there is something to change in this area: https://github.com/common-workflow-language/cwltool/blob/9781b4c89c3126e2b19f6feaf4e7327d71e754e8/cwltool/process.py#L995-L1027

Naively, I would add the following condition at line 1022:

if mx < mn:
    raise ValidationException(
        f"{a}Min cannot be greater than {a}Max."
    )

If that sounds good to the CWL experts, we can come up with a PR (and a unit test).

Cheers

[mr-c]

Hello @aldbr !

If that sounds good to the CWL experts, we can come up with a PR (and a unit test). Yes please, that works. As a bonus it would be nice to catch these types of error if static values or easily computable CWL expression are used, before the workflow is executed (cwltool --validate)

[cwl-bot]

This issue has been mentioned on Common Workflow Language Discourse. There might be relevant details there:

https://cwl.discourse.group/t/how-does-resourcerequirement-work/1019/6

[Stellatsuu]

Hello @mr-c,

I'm currently working on adding the minmax resreq validation in cwltool (I already created a pull request (see last thread update) about that issue on cwl-utils). We think it would be great to validate it on both cwl-utils and cwltool, what are your thoughs on this?

As of now, I am trying to add the validation to the --validate option, but I don't understand how that works. Do I have to manually add the validation in resolve_and_validate_document function? Or is there another way to do so?

[mr-c]

Hello @mr-c,

Howdy @Stellatsuu !

I'm currently working on adding the minmax resreq validation in cwltool (I already created a pull request (see last thread update) about that issue on cwl-utils). We think it would be great to validate it on both cwl-utils and cwltool, what are your thoughs on this?

Sure! Though there isn't much (any?) validation code in cwl-utils outside of the autogenerated code.

As of now, I am trying to add the validation to the --validate option, but I don't understand how that works. Do I have to manually add the validation in resolve_and_validate_document function? Or is there another way to do so?

Yes, the other place additional validation can go is the workflow checker in cwltool/checker.py