Skip to content

Commit 93cca90

Browse files
KedoKudoCopilot
andauthored
Quality of life improvement (#142)
* robust handling of white space delimenator * improved demo notebook for using PLEIADES at VENUS * update demo notebook to use proper visualization func * enable figure customization * Implement placeholders and TODOs for SAMMY and PLEIADES modules * Refactor: Remove unused example code and async context manager from SammyRunner; add TODOs for background, detector efficiency, last card, and resolution parameter parsing and formatting * Enhance type hints for function signatures in lpt_manager, models, config, and helper modules * Refactor: Improve exception handling in file deletion and parsing functions * fix potential security risk (shell injection) * Update src/pleiades/utils/helper.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 1759e45 commit 93cca90

File tree

26 files changed

+2428
-901
lines changed

26 files changed

+2428
-901
lines changed

examples/Notebooks/ornl_venus/au_analysis.ipynb

Lines changed: 725 additions & 0 deletions
Large diffs are not rendered by default.

examples/Notebooks/ornl_venus/hf_analysis.ipynb

Lines changed: 773 additions & 0 deletions
Large diffs are not rendered by default.

examples/Notebooks/ornl_venus/ta_analysis.ipynb

Lines changed: 706 additions & 0 deletions
Large diffs are not rendered by default.

examples/Notebooks/pleiades_venus_demo.ipynb

Lines changed: 0 additions & 761 deletions
This file was deleted.

src/pleiades/__main__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66

77
def main():
8-
print("Placeholder for PLEIADES main function")
8+
# TODO: Implement PLEIADES CLI
9+
raise NotImplementedError("PLEIADES CLI not yet implemented")
910

1011

1112
if __name__ == "__main__":

src/pleiades/nuclear/manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ def clear_cache(self, method: Optional[DataRetrievalMethod] = None, library: Opt
144144
# Call unlink directly on the file path
145145
file.unlink()
146146
logger.info(f"Deleted cached file: {file}")
147-
except Exception as e:
147+
except (OSError, PermissionError) as e:
148148
logger.error(f"Failed to delete {file}: {str(e)}")
149149

150150
def _get_data_from_direct(

src/pleiades/results/models.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ class AbundanceInfo(BaseModel):
1818

1919
class BackgroundInfo(BaseModel):
2020
# TODO: Add attributes and methods for background information
21-
pass
21+
...
2222

2323

2424
class NormalizationInfo(BaseModel):
2525
# TODO: Add attributes and methods for normalization information
26-
pass
26+
...
2727

2828

2929
class PixelInfo(BaseModel):

src/pleiades/sammy/backends/local.py

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
#!/usr/bin/env python
22
"""Local backend implementation for SAMMY execution."""
33

4-
import shlex
4+
import os
55
import subprocess
6-
import textwrap
76
from datetime import datetime
87
from pathlib import Path
98
from typing import List, Union
@@ -22,14 +21,6 @@
2221

2322
logger = loguru_logger.bind(name=__name__)
2423

25-
# Known SAMMY output file patterns
26-
SAMMY_OUTPUT_FILES = {
27-
"SAMMY.LPT", # Log file
28-
"SAMMIE.ODF", # Output data file
29-
"SAMNDF.PAR", # Updated parameter file
30-
"SAMRESOLVED.PAR", # Additional parameter file
31-
}
32-
3324

3425
class LocalSammyRunner(SammyRunner):
3526
"""Implementation of SAMMY runner for local installation."""
@@ -111,36 +102,34 @@ def execute_sammy(self, files: Union[SammyFiles, SammyFilesMultiMode]) -> SammyE
111102
logger.debug(f"Working directory: {self.config.working_dir}")
112103

113104
# Generate command based on file type
105+
# Prepare input text for SAMMY based on mode
114106
if isinstance(files, SammyFilesMultiMode):
115-
# JSON mode command format
116-
sammy_command = textwrap.dedent(f"""\
117-
{self.config.sammy_executable} <<EOF
118-
{shlex.quote(files.input_file.name)}
119-
#file {shlex.quote(files.json_config_file.name)}
120-
{shlex.quote(files.data_file.name)}
121-
122-
EOF""")
123-
logger.debug("Using JSON mode command format")
107+
# JSON mode input format
108+
sammy_input = f"{files.input_file.name}\n#file {files.json_config_file.name}\n{files.data_file.name}\n\n"
109+
logger.debug("Using JSON mode input format")
124110
else:
125-
# Traditional mode command format
126-
sammy_command = textwrap.dedent(f"""\
127-
{self.config.sammy_executable} <<EOF
128-
{shlex.quote(files.input_file.name)}
129-
{shlex.quote(files.parameter_file.name)}
130-
{shlex.quote(files.data_file.name)}
131-
132-
EOF""")
133-
logger.debug("Using traditional mode command format")
111+
# Traditional mode input format
112+
sammy_input = f"{files.input_file.name}\n{files.parameter_file.name}\n{files.data_file.name}\n\n"
113+
logger.debug("Using traditional mode input format")
134114

135115
try:
116+
# Ensure libcrypto.so.1.1 is found by adding /usr/lib64 to LD_LIBRARY_PATH
117+
env = dict(os.environ)
118+
env.update(self.config.env_vars)
119+
if "LD_LIBRARY_PATH" in env:
120+
env["LD_LIBRARY_PATH"] = f"/usr/lib64:{env['LD_LIBRARY_PATH']}"
121+
else:
122+
env["LD_LIBRARY_PATH"] = "/usr/lib64"
123+
124+
# Use safer subprocess call without shell
136125
process = subprocess.run(
137-
sammy_command,
138-
shell=True,
139-
executable=str(self.config.shell_path),
140-
env=self.config.env_vars,
126+
[str(self.config.sammy_executable)],
127+
input=sammy_input,
128+
shell=False,
129+
text=True,
130+
env=env,
141131
cwd=str(self.config.working_dir),
142132
capture_output=True,
143-
text=True,
144133
)
145134

146135
end_time = datetime.now()

src/pleiades/sammy/data/options.py

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
logger = loguru_logger.bind(name=__name__)
1414

15+
# Plot constants
16+
RESIDUAL_YLIM = (-1, 1) # Y-axis limits for residual plots
17+
HISTOGRAM_BIN_RANGE = (-1, 1, 0.01) # Range and step for histogram bins
18+
1519

1620
class DataTypeOptions(str, Enum):
1721
TRANSMISSION = "TRANSMISSION"
@@ -108,21 +112,44 @@ def validate_columns(self):
108112
if col in self.data.columns:
109113
raise ValueError(f"Unexpected transmission column for cross-section data: {col}")
110114

111-
def plot_transmission(self, show_diff=False, plot_uncertainty=False):
115+
def plot_transmission(
116+
self,
117+
show_diff=False,
118+
plot_uncertainty=False,
119+
figsize=None,
120+
title=None,
121+
xscale="linear",
122+
yscale="linear",
123+
data_color="#433E3F",
124+
final_color="#ff6361",
125+
show=True,
126+
):
112127
"""
113128
Plot the transmission data and optionally the residuals.
114129
115130
Args:
116131
show_diff (bool): If True, plot the residuals.
117132
plot_uncertainty (bool): (Unused, for compatibility)
133+
figsize (tuple): Figure size (width, height) in inches.
134+
title (str): Plot title.
135+
xscale (str): X-axis scale ('linear' or 'log').
136+
yscale (str): Y-axis scale ('linear' or 'log').
137+
data_color (str): Color for experimental data points.
138+
final_color (str): Color for fitted theoretical curve.
139+
show (bool): If True, display the plot. If False, return figure object.
140+
141+
Returns:
142+
matplotlib.figure.Figure: The figure object if show=False, None otherwise.
118143
"""
119144
if self.data is None:
120145
raise ValueError("No data loaded to plot.")
121146

122147
data = self.data
123-
data_color = "#433E3F"
124148
initial_color = "#003f5c"
125-
final_color = "#ff6361"
149+
150+
# Use provided figsize or default
151+
if figsize is None:
152+
figsize = (8, 6)
126153

127154
# Column name mapping for compatibility
128155
col_exp = "Experimental transmission (dimensionless)"
@@ -135,12 +162,12 @@ def plot_transmission(self, show_diff=False, plot_uncertainty=False):
135162
2,
136163
2,
137164
sharey=False,
138-
figsize=(8, 6),
165+
figsize=figsize,
139166
gridspec_kw={"width_ratios": [5, 1], "height_ratios": [5, 2]},
140167
)
141168
ax = np.ravel(ax)
142169
else:
143-
fig, ax = plt.subplots(figsize=(8, 6))
170+
fig, ax = plt.subplots(figsize=figsize)
144171
ax = [ax]
145172

146173
# Plot experimental transmission as scatter with error bars if available
@@ -165,11 +192,20 @@ def plot_transmission(self, show_diff=False, plot_uncertainty=False):
165192
color=final_color,
166193
lw=1,
167194
)
195+
# Apply scale settings first
196+
ax[0].set_xscale(xscale)
197+
ax[0].set_yscale(yscale)
198+
199+
# Then remove x-axis labels and ticks (for show_diff layout)
168200
ax[0].set_xlabel("")
169201
ax[0].set_xticks([])
170202
ax[0].legend(["data", "final fit"])
171203
ax[0].set_ylabel("transmission")
172204

205+
# Apply title if provided
206+
if title:
207+
ax[0].set_title(title)
208+
173209
# Determine y-axis limits
174210
max_y = data[col_exp].max()
175211
min_y = data[col_exp].min()
@@ -202,13 +238,15 @@ def plot_transmission(self, show_diff=False, plot_uncertainty=False):
202238
)
203239
ax[2].set_ylabel("residuals\n(fit-data)/err [σ]")
204240
ax[2].set_xlabel("energy [eV]")
205-
ax[2].set_ylim(-1, 1)
241+
ax[2].set_ylim(*RESIDUAL_YLIM)
242+
# Apply same x-scale to residual plot
243+
ax[2].set_xscale(xscale)
206244

207245
# Plot histograms of residuals
208246
if "residual_initial" in data.columns:
209247
data.plot.hist(
210248
y=["residual_initial"],
211-
bins=np.arange(-1, 1, 0.01),
249+
bins=np.arange(*HISTOGRAM_BIN_RANGE),
212250
ax=ax[3],
213251
orientation="horizontal",
214252
legend=False,
@@ -235,7 +273,12 @@ def plot_transmission(self, show_diff=False, plot_uncertainty=False):
235273
ax[3].spines["left"].set_visible(False)
236274

237275
plt.subplots_adjust(wspace=0.003, hspace=0.03)
238-
plt.show()
276+
277+
if show:
278+
plt.show()
279+
return None
280+
else:
281+
return fig
239282

240283
def plot_cross_section(self, show_diff=False, plot_uncertainty=False):
241284
"""Plot the cross-section data."""

src/pleiades/sammy/fitting/config.py

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,3 @@ def append_isotope_from_string(self, isotope_string: str) -> None:
4848
self.nuclear_params.isotopes.append(isotope_info)
4949
else:
5050
logger.error(f"Could not append Isotope {isotope_string}.")
51-
52-
53-
# example usage
54-
if __name__ == "__main__":
55-
example_config = FitConfig(
56-
fit_title="Example SAMMY Fit",
57-
tolerance=1e-5,
58-
max_iterations=100,
59-
i_correlation=10,
60-
max_cpu_time=3600.0,
61-
max_wall_time=7200.0,
62-
max_memory=8.0,
63-
max_disk=100.0,
64-
nuclear_params=nuclearParameters(),
65-
physics_params=PhysicsParameters(),
66-
data_params=SammyData(),
67-
options_and_routines=FitOptions(),
68-
)
69-
70-
print(example_config)

0 commit comments

Comments
 (0)