Skip to content

Commit 47d178e

Browse files
DavidT3DavidT3
authored
Merge pull request #1139 from DavidT3/refine/moreInformativeNonSASErrorMessages
Refine/more informative non sas error messages
2 parents fcf6d32 + 84438e9 commit 47d178e

File tree

2 files changed

+111
-79
lines changed

2 files changed

+111
-79
lines changed

‎xga/sas/run.py

Lines changed: 85 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# This code is a part of X-ray: Generate and Analyse (XGA), a module designed for the XMM Cluster Survey (XCS).
2-
# Last modified by David J Turner (turne540@msu.edu) 21/02/2024, 09:57. Copyright (c) The Contributors
2+
# Last modified by David J Turner (turne540@msu.edu) 01/05/2024, 15:27. Copyright (c) The Contributors
33

44
from functools import wraps
55
from multiprocessing.dummy import Pool
@@ -32,58 +32,74 @@ def execute_cmd(cmd: str, p_type: str, p_path: list, extra_info: dict, src: str)
3232
:return: The product object, and the string representation of the associated source object.
3333
:rtype: Tuple[BaseProduct, str]
3434
"""
35-
out, err = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE).communicate()
36-
out = out.decode("UTF-8", errors='ignore')
37-
err = err.decode("UTF-8", errors='ignore')
38-
39-
# This part for defining an image object used to make sure that the src wasn't a NullSource, as defining product
40-
# objects is wasteful considering the purpose of a NullSource, but generating exposure maps requires a
41-
# pre-existing image
42-
if p_type == "image":
43-
# Maybe let the user decide not to raise errors detected in stderr
44-
prod = Image(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd, extra_info["lo_en"],
45-
extra_info["hi_en"])
46-
if "psf_corr" in extra_info and extra_info["psf_corr"]:
47-
prod.psf_corrected = True
48-
prod.psf_bins = extra_info["psf_bins"]
49-
prod.psf_model = extra_info["psf_model"]
50-
prod.psf_iterations = extra_info["psf_iter"]
51-
prod.psf_algorithm = extra_info["psf_algo"]
52-
elif p_type == "expmap":
53-
prod = ExpMap(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd,
54-
extra_info["lo_en"], extra_info["hi_en"])
55-
elif p_type == "ccf" and "NullSource" not in src:
56-
# ccf files may not be destined to spend life as product objects, but that doesn't mean
57-
# I can't take momentarily advantage of the error parsing I built into the product classes
58-
prod = BaseProduct(p_path[0], "", "", out, err, cmd)
59-
elif (p_type == "spectrum" or p_type == "annular spectrum set components") and "NullSource" not in src:
60-
prod = Spectrum(p_path[0], extra_info["rmf_path"], extra_info["arf_path"], extra_info["b_spec_path"],
61-
extra_info['central_coord'], extra_info["inner_radius"], extra_info["outer_radius"],
62-
extra_info["obs_id"], extra_info["instrument"], extra_info["grouped"], extra_info["min_counts"],
63-
extra_info["min_sn"], extra_info["over_sample"], out, err, cmd, extra_info["from_region"],
64-
extra_info["b_rmf_path"], extra_info["b_arf_path"])
65-
elif p_type == "psf" and "NullSource" not in src:
66-
prod = PSFGrid(extra_info["files"], extra_info["chunks_per_side"], extra_info["model"],
67-
extra_info["x_bounds"], extra_info["y_bounds"], extra_info["obs_id"],
68-
extra_info["instrument"], out, err, cmd)
69-
elif p_type == 'light curve' and "NullSource" not in src:
70-
prod = LightCurve(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd,
71-
extra_info['central_coord'], extra_info["inner_radius"], extra_info["outer_radius"],
72-
extra_info["lo_en"], extra_info["hi_en"], extra_info['time_bin'], extra_info['pattern'],
73-
extra_info["from_region"])
74-
elif p_type == "cross arfs":
75-
prod = BaseProduct(p_path[0], extra_info['obs_id'], extra_info['inst'], out, err, cmd, extra_info)
76-
elif "NullSource" in src:
77-
prod = None
78-
else:
79-
raise NotImplementedError("Not implemented yet")
80-
81-
# An extra step is required for annular spectrum set components
82-
if p_type == "annular spectrum set components":
83-
prod.annulus_ident = extra_info["ann_ident"]
84-
prod.set_ident = extra_info["set_ident"]
85-
86-
return prod, src
35+
try:
36+
out, err = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE).communicate()
37+
out = out.decode("UTF-8", errors='ignore')
38+
err = err.decode("UTF-8", errors='ignore')
39+
40+
# This part for defining an image object used to make sure that the src wasn't a NullSource, as defining product
41+
# objects is wasteful considering the purpose of a NullSource, but generating exposure maps requires a
42+
# pre-existing image
43+
if p_type == "image":
44+
# Maybe let the user decide not to raise errors detected in stderr
45+
prod = Image(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd, extra_info["lo_en"],
46+
extra_info["hi_en"])
47+
if "psf_corr" in extra_info and extra_info["psf_corr"]:
48+
prod.psf_corrected = True
49+
prod.psf_bins = extra_info["psf_bins"]
50+
prod.psf_model = extra_info["psf_model"]
51+
prod.psf_iterations = extra_info["psf_iter"]
52+
prod.psf_algorithm = extra_info["psf_algo"]
53+
elif p_type == "expmap":
54+
prod = ExpMap(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd,
55+
extra_info["lo_en"], extra_info["hi_en"])
56+
elif p_type == "ccf" and "NullSource" not in src:
57+
# ccf files may not be destined to spend life as product objects, but that doesn't mean
58+
# I can't take momentarily advantage of the error parsing I built into the product classes
59+
prod = BaseProduct(p_path[0], "", "", out, err, cmd)
60+
elif (p_type == "spectrum" or p_type == "annular spectrum set components") and "NullSource" not in src:
61+
prod = Spectrum(p_path[0], extra_info["rmf_path"], extra_info["arf_path"], extra_info["b_spec_path"],
62+
extra_info['central_coord'], extra_info["inner_radius"], extra_info["outer_radius"],
63+
extra_info["obs_id"], extra_info["instrument"], extra_info["grouped"],
64+
extra_info["min_counts"], extra_info["min_sn"], extra_info["over_sample"], out, err, cmd,
65+
extra_info["from_region"], extra_info["b_rmf_path"], extra_info["b_arf_path"])
66+
elif p_type == "psf" and "NullSource" not in src:
67+
prod = PSFGrid(extra_info["files"], extra_info["chunks_per_side"], extra_info["model"],
68+
extra_info["x_bounds"], extra_info["y_bounds"], extra_info["obs_id"],
69+
extra_info["instrument"], out, err, cmd)
70+
elif p_type == 'light curve' and "NullSource" not in src:
71+
prod = LightCurve(p_path[0], extra_info["obs_id"], extra_info["instrument"], out, err, cmd,
72+
extra_info['central_coord'], extra_info["inner_radius"], extra_info["outer_radius"],
73+
extra_info["lo_en"], extra_info["hi_en"], extra_info['time_bin'], extra_info['pattern'],
74+
extra_info["from_region"])
75+
elif p_type == "cross arfs":
76+
prod = BaseProduct(p_path[0], extra_info['obs_id'], extra_info['inst'], out, err, cmd, extra_info)
77+
elif "NullSource" in src:
78+
prod = None
79+
else:
80+
raise NotImplementedError("Not implemented yet")
81+
82+
# An extra step is required for annular spectrum set components
83+
if p_type == "annular spectrum set components":
84+
prod.annulus_ident = extra_info["ann_ident"]
85+
prod.set_ident = extra_info["set_ident"]
86+
87+
return prod, src
88+
89+
# This is deliberately an all encompassing except - as I want to modify the message of what error may get thrown
90+
# and then I will re-raise it, just it will include the source, ObsID, and instrument that caused the issue
91+
except Exception as err:
92+
# Some possible errors (I'm looking at you OSError) tend to have a number as their first argument and then
93+
# the actual message as the second. In most cases though, I think just the first is populated
94+
if len(err.args) == 1:
95+
err.args = (err.args[0] + "- {s} is the associated source, the specific data used is " \
96+
"{o}-{i}.".format(s=src, o=extra_info["obs_id"], i=extra_info["instrument"]), )
97+
# But if there are two we do want to include them both
98+
else:
99+
err.args = (err.args[0], err.args[1] + "- {s} is the associated source, the specific data used is "
100+
"{o}-{i}.".format(s=src, o=extra_info["obs_id"],
101+
i=extra_info["instrument"]))
102+
raise err
87103

88104

89105
def sas_call(sas_func):
@@ -178,8 +194,22 @@ def err_callback(err):
178194
"""
179195
nonlocal raised_errors
180196
nonlocal gen
197+
nonlocal src_lookup
198+
nonlocal sources
181199

182200
if err is not None:
201+
# We used a memory address laden source name representation when we adjusted the error
202+
# message in execute_cmd, so we'll replace it with an actual name here
203+
# Again it matters how many arguments the error has
204+
if len(err.args) == 1:
205+
err_src_rep = err.args[0].split(' is the associated source')[0].split('- ')[-1].strip()
206+
act_src_name = sources[src_lookup[err_src_rep]].name
207+
err.args = (err.args[0].replace(err_src_rep, act_src_name),)
208+
else:
209+
err_src_rep = err.args[1].split(' is the associated source')[0].split('- ')[-1].strip()
210+
act_src_name = sources[src_lookup[err_src_rep]].name
211+
err.args = (err.args[0], err.args[1].replace(err_src_rep, act_src_name))
212+
183213
# Rather than throwing an error straight away I append them all to a list for later.
184214
raised_errors.append(err)
185215
gen.update(1)
@@ -270,8 +300,8 @@ def err_callback(err):
270300
sources[ind].update_products(ann_spec)
271301

272302
# Errors raised here should not be to do with SAS generation problems, but other purely pythonic errors
273-
for error in raised_errors:
274-
raise error
303+
if len(raised_errors) != 0:
304+
raise Exception(raised_errors)
275305

276306
# And here are all the errors during SAS generation, if any
277307
if len(all_to_raise) != 0:

‎xga/sources/base.py

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# This code is a part of X-ray: Generate and Analyse (XGA), a module designed for the XMM Cluster Survey (XCS).
2-
# Last modified by David J Turner (turne540@msu.edu) 08/12/2023, 15:22. Copyright (c) The Contributors
2+
# Last modified by David J Turner (turne540@msu.edu) 02/05/2024, 10:40. Copyright (c) The Contributors
33

44
import os
55
import pickle
@@ -2180,17 +2180,17 @@ def _interloper_sas_string(reg: EllipseSkyRegion, im: Image, output_unit: Union[
21802180
conv_cen = im.coord_conv(cen, output_unit)
21812181
# Have to divide the width by two, I need to know the half-width for SAS regions, then convert
21822182
# from degrees to XMM sky coordinates using the factor we calculated in the main function
2183-
w = reg.width.to('deg').value / 2 / sky_to_deg
2183+
w = (reg.width.to('deg').value / 2 / sky_to_deg).round(4)
21842184
# We do the same for the height
2185-
h = reg.height.to('deg').value / 2 / sky_to_deg
2185+
h = (reg.height.to('deg').value / 2 / sky_to_deg).round(4)
21862186
if w == h:
21872187
shape_str = "(({t}) IN circle({cx},{cy},{r}))"
2188-
shape_str = shape_str.format(t=c_str, cx=conv_cen[0].value, cy=conv_cen[1].value, r=h)
2188+
shape_str = shape_str.format(t=c_str, cx=conv_cen[0].round(4).value, cy=conv_cen[1].round(4).value, r=h)
21892189
else:
21902190
# The rotation angle from the region object is in degrees already
2191-
shape_str = "(({t}) IN ellipse({cx},{cy},{w},{h},{rot}))".format(t=c_str, cx=conv_cen[0].value,
2192-
cy=conv_cen[1].value, w=w, h=h,
2193-
rot=reg.angle.value)
2191+
shape_str = "(({t}) IN ellipse({cx},{cy},{w},{h},{rot}))".format(t=c_str, cx=conv_cen[0].round(4).value,
2192+
cy=conv_cen[1].round(4).value, w=w, h=h,
2193+
rot=reg.angle.round(4).value)
21942194
return shape_str
21952195

21962196
def get_annular_sas_region(self, inner_radius: Quantity, outer_radius: Quantity, obs_id: str, inst: str,
@@ -2270,30 +2270,32 @@ def get_annular_sas_region(self, inner_radius: Quantity, outer_radius: Quantity,
22702270
if inner_radius.isscalar and inner_radius.value != 0:
22712271
# And we need to define a SAS string for the actual region of interest
22722272
sas_source_area = "(({t}) IN annulus({cx},{cy},{ri},{ro}))"
2273-
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].value,
2274-
cy=xmm_central_coord[1].value, ri=inner_radius.value/sky_to_deg,
2275-
ro=outer_radius.value/sky_to_deg)
2273+
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].round(4).value,
2274+
cy=xmm_central_coord[1].round(4).value,
2275+
ri=(inner_radius.value/sky_to_deg).round(4),
2276+
ro=(outer_radius.value/sky_to_deg).round(4))
22762277
# If the inner radius is zero then we write a circle region, because it seems that's a LOT faster in SAS
22772278
elif inner_radius.isscalar and inner_radius.value == 0:
22782279
sas_source_area = "(({t}) IN circle({cx},{cy},{r}))"
2279-
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].value,
2280-
cy=xmm_central_coord[1].value,
2281-
r=outer_radius.value/sky_to_deg)
2280+
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].round(4).value,
2281+
cy=xmm_central_coord[1].round(4).value,
2282+
r=(outer_radius.value/sky_to_deg).round(4))
22822283
elif not inner_radius.isscalar and inner_radius[0].value != 0:
22832284
sas_source_area = "(({t}) IN elliptannulus({cx},{cy},{wi},{hi},{wo},{ho},{rot},{rot}))"
2284-
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].value,
2285-
cy=xmm_central_coord[1].value,
2286-
wi=inner_radius[0].value/sky_to_deg,
2287-
hi=inner_radius[1].value/sky_to_deg,
2288-
wo=outer_radius[0].value/sky_to_deg,
2289-
ho=outer_radius[1].value/sky_to_deg, rot=rot_angle.to('deg').value)
2285+
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].round(4).value,
2286+
cy=xmm_central_coord[1].round(4).value,
2287+
wi=(inner_radius[0].value/sky_to_deg).round(4),
2288+
hi=(inner_radius[1].value/sky_to_deg).round(4),
2289+
wo=(outer_radius[0].value/sky_to_deg).round(4),
2290+
ho=(outer_radius[1].value/sky_to_deg).round(4),
2291+
rot=rot_angle.to('deg').round(4).value)
22902292
elif not inner_radius.isscalar and inner_radius[0].value == 0:
22912293
sas_source_area = "(({t}) IN ellipse({cx},{cy},{w},{h},{rot}))"
2292-
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].value,
2293-
cy=xmm_central_coord[1].value,
2294-
w=outer_radius[0].value / sky_to_deg,
2295-
h=outer_radius[1].value / sky_to_deg,
2296-
rot=rot_angle.to('deg').value)
2294+
sas_source_area = sas_source_area.format(t=c_str, cx=xmm_central_coord[0].round(4).value,
2295+
cy=xmm_central_coord[1].round(4).value,
2296+
w=(outer_radius[0].value / sky_to_deg).round(4),
2297+
h=(outer_radius[1].value / sky_to_deg).round(4),
2298+
rot=rot_angle.to('deg').round(4).value)
22972299

22982300
# Combining the source region with the regions we need to cut out
22992301
if len(sas_interloper) == 0:

0 commit comments

Comments
 (0)