Skip to content

Conversation

@minxu74
Copy link
Contributor

@minxu74 minxu74 commented Aug 17, 2025

Description

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

@minxu74 minxu74 requested a review from lewisjared August 19, 2025 20:46
@codecov
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 58.90411% with 30 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ckages/climate-ref/src/climate_ref/executor/hpc.py 58.90% 26 Missing and 4 partials ⚠️
Files with missing lines Coverage Δ
...ckages/climate-ref/src/climate_ref/executor/hpc.py 62.43% <58.90%> (+5.10%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@lewisjared lewisjared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes, but nice stuff. Should we do something similar for PBS?

Comment on lines +45 to +61
username: str
partition: str | None = None
log_dir: str = "runinfo"
qos: str | None = None
req_nodes: Annotated[int, Field(strict=True, ge=1, le=1000)] = 1
cores_per_worker: Annotated[int, Field(strict=True, ge=1, le=1000)] = 1
mem_per_worker: Annotated[float, Field(strict=True, gt=0, lt=1000.0)] | None = None
max_workers_per_node: Annotated[int, Field(strict=True, ge=1, le=1000)] = 16
validation: StrictBool = False
walltime: str = "00:30:00"
scheduler_options: str = ""
retries: Annotated[int, Field(strict=True, ge=1, le=3)] = 2
max_blocks: Annotated[int, Field(strict=True, ge=1)] = 1 # one block mean one job?
worker_init: str = ""
overrides: str = ""
cmd_timeout: Annotated[int, Field(strict=True, ge=0)] = 120
cpu_affinity: str = "none"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have documentation for these fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of them are from parsl directly. We could document the fields from us and refer others to the parsl documents? What do you think of it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok. Maybe add a link to the parsl docs to make it easier to find this linkage

@lewisjared lewisjared merged commit 88de9e1 into main Oct 6, 2025
9 of 10 checks passed
@lewisjared lewisjared deleted the validate_slurm_config branch October 6, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants