Skip to content

ElPh Code Refactoring#11

Draft
PuffyDucks wants to merge 13 commits intomoule-group:mainfrom
PuffyDucks:main
Draft

ElPh Code Refactoring#11
PuffyDucks wants to merge 13 commits intomoule-group:mainfrom
PuffyDucks:main

Conversation

@PuffyDucks
Copy link
Contributor

Many changes being made for this PR.

Main Changes

  • ElPh now sends Slurm jobs by default
  • More print messages for currently running and skipped calculations
  • --overwrite flag added
  • Phasing out nmol hard coding

Details

Arguments

  • Use -l to run ElPh on local machine
  • Flags --gpu, --account, --time, --hpc have been added to modify Slum script generation
  • --homo and --svd are commented out due to never being used in the code
  • --overwrite flag added (explained in 'Workflow')
  • main.py now validates --workflow and --nmol at start
    • workflow must be {1, 2, 3}. nmol is currently restricted to {3, 4} (can possibly go to 6 without bugs? untested.)

Workflow

  • ElPh no longer skips full workflow when final files are present
  • Individual steps are still skipped when their output files are detected
    • The --overwrite flag prevents any steps from being skipped
  • Workflow skips and their file path are printed out for transparency
  • Most calculations will print a message before and/or after running
  • Above changes are currently implemented for workflow 1

Improving code quality

  • Repetitive/hard-coded logic is to be replaced
    • The most common is if nmol == 3: ... if nmol == 4: ...
    • Functions are often called repeatedly with predictable changes to arguments
  • For loops now used to make the code more flexible and easier to maintain
  • Like above, changes are only committed to workflow 1 at the moment

pathlib.Path

  • Workflow 1 code has been modified to use pathlib.Path in place of os.path
    • Not a huge deal, but makes the code nicer to read
    • A bit safer and easier

Error throwing

  • ut.print_error(message) has been changed to ut.throw_error(message, exit_status=1)
    • the function now runs sys.exit(exit_status)

@Raykang35 Raykang35 self-assigned this Sep 7, 2025
Delete svdqpts since it is not necessary
""" Using glob function in python to find the structure
file in the current path
The type of the structure files: ".cif"
def getGeometry(path: Path):
Copy link
Collaborator

@Raykang35 Raykang35 Sep 7, 2025

Choose a reason for hiding this comment

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

In mapping section, the atom order in POSCAR is different than the atom order in .cif. I am not 100% sure whether .cif file would return the same output as POSCAR. There are 2 ways we can deal with this.

  1. I suggest here we only apply POSCAR. We should add a function that if .cif, convert into POSCAR.
  2. It is worth testing with simple material simulation such as BTBT. If the output is the same, leave it as it is.

phonon_path = base_path / "2-phonons"

# Geometry file
try:
Copy link
Collaborator

@Raykang35 Raykang35 Sep 7, 2025

Choose a reason for hiding this comment

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

I don't think we need any geometry file from 2-phonons folder. The geometry file in 2-phonons is different than original geometry. It is optimized.

@Raykang35
Copy link
Collaborator

The ideas of improving the code quality and workflow is great. Let's improve workflow 2 and 3 as well.

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