-
Notifications
You must be signed in to change notification settings - Fork 4
refactor/core cleanup #3
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
Changes from all commits
f1d2e1b
6f0e0f0
5f12a88
553a59c
79fd36b
58d646c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,34 +5,50 @@ | |||||||
| import os | ||||||||
| import itertools | ||||||||
| import gzip | ||||||||
| import logging | ||||||||
|
|
||||||||
| from symwannier.nnkp import Nnkp | ||||||||
| from symwannier.sym import Sym | ||||||||
| from symwannier.io_utils import open_text_or_gz | ||||||||
|
|
||||||||
| class Amn(): | ||||||||
| """ | ||||||||
| amn file | ||||||||
| """Reader and processor for Amn files | ||||||||
| Amn(k) = <psi_mk|g_n> | ||||||||
| """ | ||||||||
| def __init__(self, file_amn, nnkp, sym = None): | ||||||||
|
|
||||||||
| def __init__(self, file_amn, nnkp, sym = None, log=None): | ||||||||
| """Load Amn data from file and prepare symmetry handling. | ||||||||
|
|
||||||||
| Parameters | ||||||||
| ---------- | ||||||||
| file_amn : str | ||||||||
| Path to amn file (optionally gzipped). | ||||||||
| nnkp : Nnkp | ||||||||
| Parsed nnkp object providing k-point and neighbor info. | ||||||||
| sym : Sym, optional | ||||||||
| Symmetry data. If provided, Amn is symmetrized/expanded accordingly. | ||||||||
| log : logging.Logger, optional | ||||||||
| Logger to use; if not provided a module logger is created. | ||||||||
| """ | ||||||||
| self.log = log or logging.getLogger(__name__) | ||||||||
| if not self.log.handlers: | ||||||||
| logging.basicConfig(level=logging.INFO, format="%(message)s") | ||||||||
|
|
||||||||
| self.nnkp = nnkp | ||||||||
| self.sym = sym | ||||||||
|
|
||||||||
| if os.path.exists(file_amn): | ||||||||
| with open(file_amn) as fp: | ||||||||
| self._read_amn(fp) | ||||||||
| elif os.path.exists(file_amn + ".gz"): | ||||||||
| with gzip.open(file_amn + ".gz", 'rt') as fp: | ||||||||
| self._read_amn(fp) | ||||||||
| else: | ||||||||
| raise Exception("failed to read amn file: " + file_amn) | ||||||||
| fp, used_path = open_text_or_gz(file_amn, desc="amn file") | ||||||||
| self.log.debug(f"Reading amn from {used_path}") | ||||||||
| with fp: | ||||||||
| self._read_amn(fp) | ||||||||
|
|
||||||||
| def _read_amn(self, fp): | ||||||||
| print(" Reading amn file") | ||||||||
| """Read amn contents from an open file-like object.""" | ||||||||
| self.log.info("Reading amn file") | ||||||||
| lines = fp.readlines() | ||||||||
| num_bands, nk, num_wann = [ int(x) for x in lines[1].split() ] | ||||||||
| dat = np.genfromtxt(lines[2:]).reshape(nk, num_wann, num_bands, 5) | ||||||||
| flat = np.fromstring("".join(lines[2:]), sep=" ") | ||||||||
|
||||||||
| flat = np.fromstring("".join(lines[2:]), sep=" ") | |
| flat_str = " ".join(lines[2:]) | |
| flat = np.fromiter((float(val) for val in flat_str.split()), dtype=float) |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Joining all remaining lines into a single string for parsing is inefficient for large amn files. This creates a potentially very large temporary string object before parsing. Consider using np.loadtxt or np.genfromtxt directly on the file pointer (after skipping headers) to avoid the memory overhead of concatenating all lines into one string.
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger formatting string uses old-style % formatting while most other logging calls in the file use f-strings or format method. For consistency, consider using the logging module's built-in parameter substitution (e.g., self.log.info("symmetrize Gk diff1 = %.5e", diff)) consistently throughout the file.
| self.log.info("symmetrize Gk diff1 = %.5e", diff) | |
| self.log.info(f"symmetrize Gk diff1 = {diff:.5e}") |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring incorrectly formats the amn shape description. The indentation is broken - it should either be part of the main docstring description or properly formatted as a Notes section. The current format with arbitrary indentation doesn't follow NumPy docstring style guidelines.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||||||||||||||||
| """Shared I/O utilities for symwannier.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| import os | ||||||||||||||||||
| import gzip | ||||||||||||||||||
| from typing import Tuple, IO | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def open_text_or_gz(path: str, desc: str = "file") -> Tuple[IO[str], str]: | ||||||||||||||||||
| """Open a text file, falling back to gzip if ``.gz`` exists. | ||||||||||||||||||
|
|
||||||||||||||||||
| Parameters | ||||||||||||||||||
| ---------- | ||||||||||||||||||
| path : str | ||||||||||||||||||
| Base path (without .gz). | ||||||||||||||||||
|
||||||||||||||||||
| Base path (without .gz). | |
| Path to the file, with or without a ``.gz`` extension. The function | |
| first tries the given path as-is, then falls back to ``path + ".gz"``. |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation incorrectly describes the return type as IO[str], but the function uses type annotations from the typing module. The actual return type is a tuple Tuple[IO[str], str] as correctly shown in the return type annotation, but the docstring says "fp : IO[str]" and "used_path : str" in separate lines under Returns, which doesn't match standard NumPy docstring format for tuples. The Returns section should clarify this is a tuple or use the tuple notation.
| fp : IO[str] | |
| Opened file-like object in text mode. | |
| used_path : str | |
| Actual path used (plain or .gz). | |
| fp_and_path : Tuple[IO[str], str] | |
| A tuple ``(fp, used_path)`` where ``fp`` is the opened file-like | |
| object in text mode and ``used_path`` is the actual path used | |
| (plain or ``.gz``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring contains a typo: "Logger to use; if not provided a module logger is created" is missing a comma. It should read "Logger to use; if not provided, a module logger is created" for proper grammar.