[ENH]: Allow for local queue using GNU parallel #306

2024-03-25T15:55:32Z

synchon commented

2024-03-25 15:55:32 +00:00

(Migrated from github.com)

Are you requiring a new dataset or marker?

I understand this is not a marker or dataset request

Which feature do you want to include?

So far the queue section of the YAML allows to use HTCondor.

The feature requested is to include a local queue that uses GNU parallel to run, locally, using multicore architectures. This will benefit all users dealing with relatively small datasets on environments without HPC/HTC, like dual-socket and powerful workstations.

How do you imagine this integrated in junifer?

By allowing this:

queue:
  jobname: TestLocalQueue
  kind: local
  jobs: 4
  env:
    kind: conda
    name: junifer

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

### Are you requiring a new dataset or marker? - [X] I understand this is not a marker or dataset request ### Which feature do you want to include? So far the `queue` section of the YAML allows to use HTCondor. The feature requested is to include a `local` queue that uses GNU parallel to run, locally, using multicore architectures. This will benefit all users dealing with relatively small datasets on environments without HPC/HTC, like dual-socket and powerful workstations. ### How do you imagine this integrated in junifer? By allowing this: ```YAML queue: jobname: TestLocalQueue kind: local jobs: 4 env: kind: conda name: junifer ``` ### Do you have a sample code that implements this outside of junifer? _No response_ ### Anything else to say? _No response_

github-actions[bot] commented

2024-03-25 16:03:31 +00:00

(Migrated from github.com)

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-03-28 09:31 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-03-28 09:31 UTC

fraimondo (Migrated from github.com) requested changes

2024-03-26 10:09:55 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

Why are we changing this? Is this absolutely necessary? If someone updates junifer and then continues with a pre-existing yaml, it will have a weird junifer_jobs directory.

junifer/api/queue_context/gnu_parallel_local_adapter.py

					
				@ -0,0 +1,258 @@

				"""Define concrete class for generating GNU Parallel (local) assets."""

While this syntax is fine for small jobs, it will create a huge command line and does not scale.

A better option is to use the --arg-file argument and give a file with the arguments to use.

While this syntax is fine for small jobs, it will create a huge command line and does not scale. A better option is to use the `--arg-file` argument and give a file with the arguments to use.

We can directly add this to the command (see my comment above)

Some comments about the parallel arguments.
--bar: ok, shows a bar.
--halt: disagree, we should run as much as we can. If one subject fails, continue.
--resume: add this so the user sees the full command that can be stopped and continued (with ctrl+C)
--resume-failed: add this also, for the same reason. It will not have any effect on the first run. It will have an effect on the subsequent runs.
--joblob: perfect

Some other to consider:
--output-as-files: to redirect output to a file and keep the stdout clean
--delay: prevent having N-jobs doing the same IO operation at the beginning, which will definitely create a bottleneck and maybe a failure.

Some comments about the `parallel` arguments. `--bar`: ok, shows a bar. `--halt`: disagree, we should run as much as we can. If one subject fails, continue. `--resume`: add this so the user sees the full command that can be stopped and continued (with ctrl+C) `--resume-failed`: add this also, for the same reason. It will not have any effect on the first run. It will have an effect on the subsequent runs. `--joblob`: perfect Some other to consider: `--output-as-files`: to redirect output to a file and keep the stdout clean `--delay`: prevent having N-jobs doing the same IO operation at the beginning, which will definitely create a bottleneck and maybe a failure.

junifer/api/queue_context/queue_context_adapter.py

There's no pre-run anymore?

What if the user wants to set environmental variables even in a gnu parallel environment? Keep in mind that GNU parallel can also be used to run over SSH.

I liked the idea of having some sort of self-contained running scripts.

Imagine this scenario:

User queues with local and starts running gnu-parallel on the HCP.
Takes too long, wants to compute another thing, presses CTRL+C to stop.
Runs another thing, closes the terminal.
Opens the terminal, goes to the junifer_jobs dir and runs the parallel command again.

This should work.

There's no pre-run anymore? What if the user wants to set environmental variables even in a gnu parallel environment? Keep in mind that GNU parallel can also be used to run over SSH. I liked the idea of having some sort of self-contained running scripts. Imagine this scenario: - User queues with `local` and starts running gnu-parallel on the HCP. - Takes too long, wants to compute another thing, presses CTRL+C to stop. - Runs another thing, closes the terminal. - Opens the terminal, goes to the `junifer_jobs` dir and runs the `parallel` command again. This should work.

synchon (Migrated from github.com) reviewed

2024-03-26 12:21:08 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

If an user tries out different queueing system but with the same job name (which will most likely be the case), then this is the simplest way imo to handle the assets for each system.

synchon (Migrated from github.com) reviewed

2024-03-26 12:35:22 +00:00

junifer/api/queue_context/gnu_parallel_local_adapter.py

					
				@ -0,0 +1,258 @@

				"""Define concrete class for generating GNU Parallel (local) assets."""

Agreed. Any reason for preferring --output-as-files over --results? The latter stores the stdout, stderr and seq value in a structured way.

Agreed. Any reason for preferring `--output-as-files` over `--results`? The latter stores the stdout, stderr and seq value in a structured way.

synchon (Migrated from github.com) reviewed

2024-03-26 12:38:15 +00:00

junifer/api/queue_context/queue_context_adapter.py

There's no pre-run anymore?

I've decided to revert it in a better way.

What if the user wants to set environmental variables even in a gnu parallel environment? Keep in mind that GNU parallel can also be used to run over SSH.

Fair argument with the env vars. We'll not let users run GNU parallel over SSH yet as it's not as simple as running locally. You need to tweak file copying, output handling, cleanup and compression, a lot more moving parts. But maybe at some point in the future, we can support it.

> There's no pre-run anymore? I've decided to revert it in a better way. > What if the user wants to set environmental variables even in a gnu parallel environment? Keep in mind that GNU parallel can also be used to run over SSH. Fair argument with the env vars. We'll not let users run GNU parallel over SSH yet as it's not as simple as running locally. You need to tweak file copying, output handling, cleanup and compression, a lot more moving parts. But maybe at some point in the future, we can support it.

fraimondo (Migrated from github.com) reviewed

2024-03-26 13:31:44 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

From what I know, the junifer queue command does not allow to use an existing jobdir.

From what I know, the junifer queue command does not allow to use an existing `jobdir`.

fraimondo (Migrated from github.com) reviewed

2024-03-26 13:33:40 +00:00

junifer/api/queue_context/gnu_parallel_local_adapter.py

					
				@ -0,0 +1,258 @@

				"""Define concrete class for generating GNU Parallel (local) assets."""

Did not know of the existence of --results. Pick what you think it's the best option.

Did not know of the existence of `--results`. Pick what you think it's the best option.

synchon (Migrated from github.com) reviewed

2024-03-26 13:34:09 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

I don't quite understand how that can be a problem as the jobdir includes the queue kind dir.

I don't quite understand how that can be a problem as the `jobdir` includes the queue kind dir.

fraimondo (Migrated from github.com) reviewed

2024-03-26 13:34:30 +00:00

junifer/api/queue_context/queue_context_adapter.py

For the SSH, not officialy, but someone might "tweak" the parallel command. In any case, even if SSH is not a supported use case, just closing the terminal and opening it again should continue were it was.

synchon (Migrated from github.com) reviewed

2024-03-26 13:35:56 +00:00

junifer/api/queue_context/queue_context_adapter.py

For the SSH, not officialy, but someone might "tweak" the parallel command.

That's for the "hacker" spirited-soul :)

In any case, even if SSH is not a supported use case, just closing the terminal and opening it again should continue were it was.

Of course.

> For the SSH, not officialy, but someone might "tweak" the parallel command. That's for the "hacker" spirited-soul :) > In any case, even if SSH is not a supported use case, just closing the terminal and opening it again should continue were it was. Of course.

fraimondo (Migrated from github.com) reviewed

2024-03-26 13:36:49 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

Updating junifer creates an inconsistent junifer_jobs directory.

Updating junifer creates an inconsistent `junifer_jobs` directory.

fraimondo (Migrated from github.com) reviewed

2024-03-26 13:38:24 +00:00

junifer/api/queue_context/queue_context_adapter.py

My own personal cluster (my phd lab during the nights).

My own personal cluster (my phd lab during the nights). ![de noche](https://github.com/juaml/junifer/assets/4493699/4ce62d20-d43b-437f-823f-0b35a1c6062e)

synchon (Migrated from github.com) reviewed

2024-03-26 13:38:48 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

We can add it to the release notes and / or issue a note in the documentation. Do you have an alternative for this?

synchon (Migrated from github.com) reviewed

2024-03-26 13:40:16 +00:00

junifer/api/queue_context/queue_context_adapter.py

Can relate to the picture, don't have an anecdotal picture though :D

codecov[bot] commented

2024-03-26 15:17:23 +00:00

(Migrated from github.com)

Attention: Patch coverage is 95.60440% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 88.15%. Comparing base (f957538) to head (9ec6835).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #306      +/-   ##
==========================================
+ Coverage   87.98%   88.15%   +0.16%     
==========================================
  Files         109      111       +2     
  Lines        4728     4820      +92     
  Branches      949      957       +8     
==========================================
+ Hits         4160     4249      +89     
- Misses        415      417       +2     
- Partials      153      154       +1

Flag	Coverage Δ
docs	`100.00% <ø> (?)`
junifer	`88.15% <95.60%> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
junifer/api/queue_context/__init__.py	`100.00% <100.00%> (ø)`
junifer/api/queue_context/htcondor_adapter.py	`99.10% <ø> (ø)`
junifer/api/functions.py	`93.75% <75.00%> (+0.09%)`	⬆️
...er/api/queue_context/gnu_parallel_local_adapter.py	`96.51% <96.51%> (ø)`

... and 2 files with indirect coverage changes

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/306?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report Attention: Patch coverage is `95.60440%` with `4 lines` in your changes are missing coverage. Please review. > Project coverage is 88.15%. Comparing base [(`f957538`)](https://app.codecov.io/gh/juaml/junifer/commit/f9575386c6ff99b447168bff6f772e7bcf13d07e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`9ec6835`)](https://app.codecov.io/gh/juaml/junifer/pull/306?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/306/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #306 +/- ## ========================================== + Coverage 87.98% 88.15% +0.16% ========================================== Files 109 111 +2 Lines 4728 4820 +92 Branches 949 957 +8 ========================================== + Hits 4160 4249 +89 - Misses 415 417 +2 - Partials 153 154 +1 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/306/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/306/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (?)` | | | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/306/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `88.15% <95.60%> (+0.16%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/juaml/junifer/pull/306?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/api/queue\_context/\_\_init\_\_.py](https://app.codecov.io/gh/juaml/junifer/pull/306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvcXVldWVfY29udGV4dC9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [junifer/api/queue\_context/htcondor\_adapter.py](https://app.codecov.io/gh/juaml/junifer/pull/306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvcXVldWVfY29udGV4dC9odGNvbmRvcl9hZGFwdGVyLnB5) | `99.10% <ø> (ø)` | | | [junifer/api/functions.py](https://app.codecov.io/gh/juaml/junifer/pull/306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvZnVuY3Rpb25zLnB5) | `93.75% <75.00%> (+0.09%)` | :arrow_up: | | [...er/api/queue\_context/gnu\_parallel\_local\_adapter.py](https://app.codecov.io/gh/juaml/junifer/pull/306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvcXVldWVfY29udGV4dC9nbnVfcGFyYWxsZWxfbG9jYWxfYWRhcHRlci5weQ==) | `96.51% <96.51%> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/juaml/junifer/pull/306/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) </details>

fraimondo (Migrated from github.com) reviewed

2024-03-26 15:17:47 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

My take is that we don't need the kind.lower() part, as you won't be able to run junifer queue if the jobname directory exists. It will be, technically, a different job with a different name.

I don't see any usecase to complicate ourselves like this.

My take is that we don't need the `kind.lower()` part, as you won't be able to run `junifer queue` if the `jobname` directory exists. It will be, technically, a different job with a different name. I don't see any usecase to complicate ourselves like this.

synchon (Migrated from github.com) reviewed

2024-03-26 15:32:48 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

So if a user wants to run a YAML using GNU Parallel (for testing) on juseless for example and then switch to HTCondor (for complete run), you say that either the user changes the job name, overwrites it or creates a new YAML?

fraimondo (Migrated from github.com) reviewed

2024-03-27 15:06:48 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

The user needs to change the YAML anyways (the queue section).

Usually the testing is done with junifer run. You don't need to test the queueing mechanism unless you are doing some advanced things. If you want to test the non-interactive run, even the GNU parallel tool is not the way. You should just run junifer queue with --element to queue only one element.

The user needs to change the YAML anyways (the `queue` section). Usually the testing is done with `junifer run`. You don't need to test the _queueing mechanism_ unless you are doing some advanced things. If you want to test the non-interactive run, even the GNU parallel tool is not the way. You should just run `junifer queue` with `--element` to queue only one element.

synchon (Migrated from github.com) reviewed

2024-03-27 15:17:05 +00:00

junifer/api/functions.py

					
				@ -239,7 +239,7 @@ def queue(

				        if the ``jobdir`` exists and ``overwrite = False``.

Fair enough.

synchon commented

2024-03-27 15:29:36 +00:00

(Migrated from github.com)

@fraimondo Do we merge after CI completion?

fraimondo (Migrated from github.com) approved these changes

2024-03-28 09:23:23 +00:00

Sign in to join this conversation.

No reviewers

fraimondo

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

juaml/junifer!306

				`@ -239,7 +239,7 @@ def queue(`
				if the ``jobdir`` exists and ``overwrite = False``.

				`@ -0,0 +1,258 @@`
				`"""Define concrete class for generating GNU Parallel (local) assets."""`

Rows
Columns

[ENH]: Allow for local queue using GNU parallel #306

Are you requiring a new dataset or marker?

Which feature do you want to include?

How do you imagine this integrated in junifer?

Do you have a sample code that implements this outside of junifer?

Anything else to say?

Codecov Report

[ENH]: Allow for `local` queue using GNU parallel #306