Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add submission_script_path to write run_queue.sh script to a different location #364

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions pysqa/base/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ def submit_job(
dependency_list: Optional[List[str]] = None,
command: Optional[str] = None,
submission_template: Optional[Union[str, Template]] = None,
submission_script_path: Optional[str] = None,
**kwargs,
) -> Union[int, None]:
"""
Expand All @@ -160,6 +161,8 @@ def submit_job(
run_time_max (int/None): The maximum run time for the job.
dependency_list (list[str]/None): List of job dependencies.
command (str/None): The command to execute for the job.
submission_template (str/Template): Jinja2 template to write submission script.
submission_script_path (str/None): path to write the submission script to.

Returns:
int: The job ID.
Expand All @@ -170,7 +173,7 @@ def submit_job(
)
if submission_template is None:
submission_template = self._submission_template
working_directory, queue_script_path = self._write_queue_script(
working_directory, submission_script_path = self._write_queue_script(
queue=queue,
job_name=job_name,
working_directory=working_directory,
Expand All @@ -180,11 +183,12 @@ def submit_job(
command=command,
dependency_list=dependency_list,
submission_template=submission_template,
submission_script_path=submission_script_path,
**kwargs,
)
out = self._execute_command(
commands=self._list_command_to_be_executed(
queue_script_path=queue_script_path
submission_script_path=submission_script_path
),
working_directory=working_directory,
split_output=False,
Expand Down Expand Up @@ -297,17 +301,17 @@ def get_status_of_jobs(self, process_id_lst: List[int]) -> List[str]:
results_lst.append("finished")
return results_lst

def _list_command_to_be_executed(self, queue_script_path: str) -> list:
def _list_command_to_be_executed(self, submission_script_path: str) -> list:
"""
Get the list of commands to be executed.

Args:
queue_script_path (str): The path to the queue script.
submission_script_path (str): The path to the queue script.

Returns:
list: The list of commands to be executed.
"""
return self._commands.submit_job_command + [queue_script_path]
return self._commands.submit_job_command + [submission_script_path]

def _execute_command(
self,
Expand Down Expand Up @@ -349,6 +353,7 @@ def _write_queue_script(
run_time_max: Optional[int] = None,
dependency_list: Optional[List[int]] = None,
command: Optional[str] = None,
submission_script_path: Optional[str] = None,
**kwargs,
) -> Tuple[str, str]:
"""
Expand Down Expand Up @@ -385,10 +390,11 @@ def _write_queue_script(
)
if not os.path.exists(working_directory):
os.makedirs(working_directory)
queue_script_path = os.path.join(working_directory, "run_queue.sh")
with open(queue_script_path, "w") as f:
if submission_script_path is None:
submission_script_path = os.path.join(working_directory, "run_queue.sh")
with open(submission_script_path, "w") as f:
f.writelines(queue_script)
return working_directory, queue_script_path
return working_directory, submission_script_path

def _job_submission_template(
self,
Expand Down
6 changes: 4 additions & 2 deletions pysqa/base/modular.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def submit_job(
int: The cluster queue ID.

"""
working_directory, queue_script_path = self._write_queue_script(
working_directory, submission_script_path = self._write_queue_script(
queue=queue,
job_name=job_name,
working_directory=working_directory,
Expand All @@ -87,7 +87,9 @@ def submit_job(
cluster_module = self._queue_to_cluster_dict[queue]
commands = self._switch_cluster_command(
cluster_module=cluster_module
) + self._list_command_to_be_executed(queue_script_path=queue_script_path)
) + self._list_command_to_be_executed(
submission_script_path=submission_script_path
)
out = self._execute_command(
commands=commands,
working_directory=working_directory,
Expand Down
4 changes: 4 additions & 0 deletions pysqa/queueadapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ def submit_job(
dependency_list: Optional[List[str]] = None,
command: Optional[str] = None,
submission_template: Optional[Union[str, Template]] = None,
submission_script_path: Optional[str] = None,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Parent class signature needs to be updated

The submission_script_path parameter is not present in the parent class QueueAdapterAbstractClass in pysqa/base/abstract.py. Adding this parameter to the child class without updating the parent class violates the Liskov Substitution Principle. The abstract base class should be updated first to include this parameter.

  • pysqa/base/abstract.py: Update QueueAdapterAbstractClass.submit_job() method signature to include the submission_script_path parameter
  • pysqa/base/core.py: Update the intermediate implementation if present
🔗 Analysis chain

Verify parent class method signature

The addition of the submission_script_path parameter looks good, but we should ensure this change is consistent with the parent class QueueAdapterAbstractClass.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the parent class method signature is compatible
# Look for the abstract class definition and its submit_job method
ast-grep --pattern 'class QueueAdapterAbstractClass:
  $$$
  def submit_job($$$):
    $$$'

Length of output: 92


Script:

#!/bin/bash
# Let's try a different approach to find the parent class and its method signature
# First, let's find all Python files that might contain QueueAdapterAbstractClass
fd '.*\.py$' --exec rg -l 'QueueAdapterAbstractClass'

# Then, let's look for the actual class definition and method signature with context
rg -A 10 'class QueueAdapterAbstractClass' '*.py'

Length of output: 220


Script:

#!/bin/bash
# Now that we found the files, let's check each one for the class definition
echo "=== Checking pysqa/base/abstract.py ==="
rg -A 10 'class QueueAdapterAbstractClass' pysqa/base/abstract.py

echo "=== Checking pysqa/base/core.py ==="
rg -A 10 'class QueueAdapterAbstractClass' pysqa/base/core.py

echo "=== Checking submit_job method signature ==="
rg -B 2 -A 5 'def submit_job' pysqa/base/abstract.py pysqa/base/core.py

Length of output: 1717

**kwargs,
) -> int:
"""
Expand All @@ -212,6 +213,8 @@ def submit_job(
run_time_max (int/None): Maximum runtime in seconds (optional)
dependency_list(list[str]/None: Job ids of jobs to be completed before starting (optional)
command (str/None): shell command to run in the job
submission_template (str/Template): Jinja2 template to write submission script.
submission_script_path (str/None): path to write the submission script to.
**kwargs: allows writing additional parameters to the job submission script if they are available in the
corresponding template.

Expand All @@ -228,6 +231,7 @@ def submit_job(
dependency_list=dependency_list,
command=command,
submission_template=submission_template,
submission_script_path=submission_script_path,
**kwargs,
)

Expand Down
Loading