Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • Dec 02 21:00
    stevekm commented #3955
  • Dec 02 20:48
    adamnovak commented #3955
  • Dec 02 20:38
    adamnovak converted_to_draft #3956
  • Dec 02 20:38
    adamnovak opened #3956
  • Dec 02 20:34

    adamnovak on 3942-aws-batch-batch-system

    Implement untested AWSBatchBatc… Fix AWS Batch pylint errors Fix AWS Batch mypy errors (compare)

  • Dec 02 20:05
    stevekm commented #3955
  • Dec 02 19:58
    unito-bot edited #3955
  • Dec 02 19:57
    stevekm opened #3955
  • Dec 02 19:39
    unito-bot edited #3819
  • Dec 02 19:39
    unito-bot edited #3819
  • Dec 02 19:24
    DailyDreaming commented #3856
  • Dec 02 17:24
    mr-c commented #3954
  • Dec 02 16:58
    adamnovak opened #3954
  • Dec 02 16:55

    adamnovak on 3952-fix-check-spacing

    Change spacing on test to make … Make runnable binaries mandator… Explain some weird stuff we do and 1 more (compare)

  • Dec 02 16:46
    adamnovak assigned #3952
  • Dec 02 16:45
    adamnovak commented #3952
  • Dec 02 16:32
    mr-c synchronize #3953
  • Dec 02 16:32

    mr-c on 3943-log-metrics

    CWL logging tweaks (#3945) Rescue fast enough to protect c… Document job names & reformat t… and 1 more (compare)

  • Dec 02 16:32
    mr-c updated the wiki
  • Dec 02 16:31

    mr-c on 3744-document-job-naming

    (compare)

crusoe
@mr-c:matrix.org
[m]
Okay. What is special about that queue? Longer walltime is allowed? Special hardware? Bigmem?
Martín Beracochea
@mberacochea
bigmem
Adam Novak
@adamnovak
Yeah, Toil doesn't have this feature. If you have an idea for how to improve the LSF batch system code in Toil so that it can know what queues jobs need to go in based on their memory requirements, and you can figure out how to code it so that it will still work on everybody else's LSF clusters, we could take a PR.
Or if CWL grew a way to add an LSF queue annotation to a job, we could maybe punch a hole through to the batch system so that it could know about it.
crusoe
@mr-c:matrix.org
[m]
@mberacochea do you have to specify a queue? Toil does put the memory requirements in the batch job. You could ask your LSF admin if that'd be enough
There is an unimplemented proposal for overriding the queue for a certain job: common-workflow-language/common-workflow-language#581
Martín Beracochea
@mberacochea
Thank you both. I need to specify the queue, otherwise the LSF rejects the job (I'll see if the admins have something to sort that out).
yeah, the overrides seem to be the way to go in my case
crusoe
@mr-c:matrix.org
[m]
Okay, if they can't cope then we could try implementing a vendor extension version of BatchQueue for toil-cwl-runner
Martín Beracochea
@mberacochea
All right. Sounds like a plan. Thanks
crusoe
@mr-c:matrix.org
[m]
@adamnovak that would require a way to pass per Toil.job options to the batch systems, yes
Adam Novak
@adamnovak
In general we need to revise requirements to be a bit more free-form to support things like GPUs. I think we'll end up with some kind of dict of keys that the batch system can consult.
Michael Milton
@multimeric
If I run a Python workflow, and it completes, then I edit the workflow and re-run it with--restart, Toil still thinks it's finished and I get [2021-09-24T14:33:48+1000] [MainThread] [W] [toil.common] Requested restart but the workflow has already been completed; allowing exports to rerun.. What I actually want it to do here is cache the jobs that are unchanged and re-run those that have changed. Is this possible at all in toil?
Adam Novak
@adamnovak
@multimeric Unfortunately that's not a feature that we have. We don't keep any record of what particular Python values or class or function definitions each workflow task depends on, or what version of those definitions it ran with. In fact, we don't really keep records of the jobs at all after they and their descendants complete, and we sever the connection between a job description and its Python code after that code runs successfully, unless it's a checkpoint job.
If we wanted to do this, we'd have to basically make all jobs checkpoint jobs, except even more so because we'd have to keep them around after they and their descendants all finished. Then we'd have to come up with a new way to enumerate the jobs that still need to happen (which right now is basically 1:1 with the jobs that still exist). We'd also need to come up with a way to traverse the possible call and constant access graph of a Python function, determine an identifier for the version of each function or constant that is used, and store that along with the finished job.
And anything that accessed code via dynamic lookup would need to either always or never rerun, because we wouldn't be able to find the code.
Adam Novak
@adamnovak
That all being said, WDL runners are able to do this with WDL code, so it might not be impossible.
Michael Milton
@multimeric
Thanks for the answer. One angle I've seen used in another system is to annotate each job with a hash which we allow the user to calculate. Then the user can try simple solutions like just hashing the file that the job resides in, or alternatively just keeping a manual version number for each task.
If WDL already does this, then I guess Toil has some concept of "has this job changed", which I would just need to plug this logic into
Marcel Loose
@gmloose
Can anyone explain to me how to interpret the output of toil --stats? The documentation is quite limited.
Marcel Loose
@gmloose
Today, I've been bitten by the fact that CWLTool URL-encodes a + character is a filename to %2B. This results in a error: Cannot make job: Invalid filename: 'P233%2B35_structure.txt' contains illegal characters
I saw there are several issues that refer to this:
common-workflow-language/cwltool#1260,
common-workflow-language/cwltool#1098, and
common-workflow-language/cwltool#1445.
Where the last one even contains an almost finished pull request.
So I was wondering, what's the status of this issue? Is it indeed a bug in CWLTool, or is this a (too) strict limitation by CWLTool on allowed characters in a filename?
crusoe
@mr-c:matrix.org
[m]
Sorry to hear that @gmloose ; https://github.com/common-workflow-language/cwltool/pull/1446#issuecomment-850896086 shows that the PR needs some assistance. Would you like to finish it up?
It is indeed a bug in cwltool; and it should be fixed
Marcel Loose
@gmloose
I could have a look, though I have limited time.
crusoe
@mr-c:matrix.org
[m]
Thanks. Seems like it just needs a docstring plus tweaking Alex's tests to cover the rest of the newly added code paths
Marcel Loose
@gmloose
I was curious about the doc-string part. None of the functions in that file have a doc-string. Why is it enforced on this single test function?
crusoe
@mr-c:matrix.org
[m]
It was a requirement added later, and we use diff-cover to only enforce it for new code
Marcel Loose
@gmloose
I'm not really familiar with tox. How can I run only the modified test test_path_checks.py and get coverage stats?
crusoe
@mr-c:matrix.org
[m]

The two lines with no test coverage are annotated at https://github.com/common-workflow-language/cwltool/pull/1446/files#annotation_2008443310

For local checking you'll need to run all the tests with make diff-cover

Marcel Loose
@gmloose

Hm, that fails. ```$ make diff-cover
python --version 2>&1 | grep "Python 3"
Python 3.6.9
python -m pytest -rs --cov --cov-config=.coveragerc --cov-report=
ERROR: usage: main.py [options] [file_or_dir] [file_or_dir] [...]
main.py: error: unrecognized arguments: -n --cov --cov-config=.coveragerc --cov-report=
inifile: /home/marcel/code/cwltool/tox.ini
rootdir: /home/marcel/code/cwltool

Makefile:155: recipe for target 'testcov' failed
make: * [testcov] Error 4
```

OK, make install-dep seemed to do the trick.
Marcel Loose
@gmloose
But it still runs all tests :(. I had expected make diff-cover would only run tests in test_path_checks.py. Am I doing something wrong?
crusoe
@mr-c:matrix.org
[m]
you have to run all the tests to see how the other changes may have impacted the code coverage. You aren't doing anything wrong, no.
crusoe
@mr-c:matrix.org
[m]

So when I run make diff-cover on that PR locally I get:

cwltool/command_line_tool.py (62.5%): Missing lines 207,256-257

Which matches what codecov.io reports, so that is good 🙂
Marcel Loose
@gmloose
I've tried to get my head around what exactly is going on in revmap_file and the new test. I get the impression that the test (in its current setup) can only check that what you put in as filename, also gets out (i.e. the external filename representation). I guess that's why only the if clause is covered by the test. I guess the else clause will only be executed if you supply an internal filename representation (at least, that's what I'm guessing right now). I'm not sure how I would have to supply an internal filename representation in that current test, because it uses a CommandLineTool, which is an external thingy.
crusoe
@mr-c:matrix.org
[m]
internal in this case refers to a path within a software (docker) container
(if that works then we can collapse the code duplication later, don't worry about that for now)
Marcel Loose
@gmloose
Only adding a DockerRequirement doesn't help much. I still have an empty scheme in line 203 of command_line_tool.py. I know I can call as_uri() on the Path variable that is used in the test , but I don't know how to tweak the test to do so, without completely breaking it.
crusoe
@mr-c:matrix.org
[m]
Ah, for that set RuntimeContext.outdir to some file:/// reference to a tmpdir
(tmp_path / "outdir").as_uri()
hmm.. no, that doesn't work
That is some very old code, the check for a schema in outdir
crusoe
@mr-c:matrix.org
[m]
Toil-cwl-runner doesn't use that. Maybe arvados-cwl-runner does? Paging @tetron ..
@gmloose: Lets ignore that part for the moment (and thanks for looking into this!) ; what about the other two lines that have no coverage?
huh, even more ancient code, from January 2018..