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.

Sign up for GitHub

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

Jump to bottom

CATKIT-96: Reduce file IO overhead for sending commands to BMC #236

Merged
merged 5 commits into from
May 14, 2021
Merged

CATKIT-96: Reduce file IO overhead for sending commands to BMC #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
33358ad
Cache DM mask.
ehpor May 12, 2021
d530b1e
Cache flat maps.
ehpor May 12, 2021
350f7e8
Set get_dm_mask.mask correctly.
ehpor May 12, 2021
4d10f25
Simplify convert_dm_command_to_image() as well.
ehpor May 12, 2021
d122292
Post jnoss review changes.
ehpor May 13, 2021
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
82 changes: 46 additions & 36 deletions catkit/hardware/boston/DmCommand.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
m_per_volt_map1 = None # for caching the conversion factor, to avoid reading from disk each time
m_per_volt_map2 = None # for caching the conversion factor, to avoid reading from disk each time

flat_map_dm1 = None # for caching the conversion factor, to avoid reading from disk each time
flat_map_dm2 = None # for caching the conversion factor, to avoid reading from disk each time

class DmCommand(object):
def __init__(self, data, dm_num, flat_map=False, bias=False, as_voltage_percentage=False,
Expand Down Expand Up @@ -107,14 +109,7 @@ def to_dm_command(self):

# OR apply Flat Map (which is itself biased).
elif self.flat_map:
if self.dm_num == 1:
flat_map_file_name = CONFIG_INI.get("boston_kilo952", "flat_map_dm1")
flat_map_volts = fits.open(os.path.join(self.calibration_data_path, flat_map_file_name))
dm_command += flat_map_volts[0].data
else:
flat_map_file_name = CONFIG_INI.get("boston_kilo952", "flat_map_dm2")
flat_map_volts = fits.open(os.path.join(self.calibration_data_path, flat_map_file_name))
dm_command += flat_map_volts[0].data
dm_command += get_flat_map_volts(self.dm_num)

# Convert between 0-1.
dm_command /= self.max_volts
Expand Down Expand Up @@ -187,18 +182,33 @@ def load_dm_command(path, dm_num=1, flat_map=False, bias=False, as_volts=False):


def get_flat_map_volts(dm_num):
"""
Get the flat map for a given dm. The flat map is in volts for each actuator.
:param dm_num: Which DM to lead the command for.
:return: flat map for the selected DM. This function caches the map to avoid multiple disk access.
"""
calibration_data_package = CONFIG_INI.get("optics_lab", "calibration_data_package")
calibration_data_path = os.path.join(catkit.util.find_package_location(calibration_data_package),
"hardware",
"boston")

global flat_map_dm1, flat_map_dm2

if dm_num == 1:
flat_map_file_name = CONFIG_INI.get("boston_kilo952", "flat_map_dm1")
flat_map_volts = fits.open(os.path.join(calibration_data_path, flat_map_file_name))
return flat_map_volts[0].data
else:
flat_map_file_name = CONFIG_INI.get("boston_kilo952", "flat_map_dm2")
flat_map_volts = fits.open(os.path.join(calibration_data_path, flat_map_file_name))
return flat_map_volts[0].data
if flat_map_dm1 is None:
fname = CONFIG_INI.get("boston_kilo952", "flat_map_dm1")
flat_map_dm1 = fits.getdata(os.path.join(calibration_data_path, fname))

return flat_map_dm1

if dm_num == 2:
if flat_map_dm2 is None:
fname = CONFIG_INI.get("boston_kilo952", "flat_map_dm2")
flat_map_dm2 = fits.getdata(os.path.join(calibration_data_path, fname))

return flat_map_dm2

raise ValueError('dm_num should be either 1 or 2.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Not gonna ask you to redo this, but this sort of logic pattern is neater and more resilient to future changes when self-contained as an else to an elif, i.e.,

if dm_num == 1:
  ...
elif dm_num == 2:
  ...
else:
   raise

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered doing a dictionary approach too. That would have been even cleaner. PEP 634 is gonna be so good for this type of stuff too. I've been waiting for that for a long time.



def get_m_per_volt_map(dm_num):
Expand All @@ -213,14 +223,22 @@ def get_m_per_volt_map(dm_num):
"boston")

global m_per_volt_map1, m_per_volt_map2
if m_per_volt_map1 is None:
m_per_volt_map1 = fits.getdata(os.path.join(calibration_data_path,
CONFIG_INI.get("boston_kilo952", "gain_map_dm1")))
if m_per_volt_map2 is None:
m_per_volt_map2 = fits.getdata(os.path.join(calibration_data_path,
CONFIG_INI.get("boston_kilo952", "gain_map_dm2")))

return m_per_volt_map1 if dm_num == 1 else m_per_volt_map2
if dm_num == 1:
if m_per_volt_map1 is None:
fname = CONFIG_INI.get("boston_kilo952", "gain_map_dm1")
m_per_volt_map1 = fits.getdata(os.path.join(calibration_data_path, fname))

return m_per_volt_map1

if dm_num == 2:
if m_per_volt_map2 is None:
fname = CONFIG_INI.get("boston_kilo952", "gain_map_dm2")
m_per_volt_map2 = fits.getdata(os.path.join(calibration_data_path, fname))

return m_per_volt_map2

raise ValueError('dm_num should be either 1 or 2.')


def convert_volts_to_m(data, dm_num, meter_to_volt_map=None):
Expand Down Expand Up @@ -262,26 +280,18 @@ def convert_m_to_volts(data, dm_num, meter_to_volt_map=None):


def convert_dm_command_to_image(dm_command):
# Flatten the mask using index952
mask = catkit.util.get_dm_mask()
index952 = np.flatnonzero(mask)
mask = catkit.util.get_dm_mask().astype('bool')
number_of_actuators = np.count_nonzero(mask)

number_of_actuators_per_dimension = CONFIG_INI.getint('boston_kilo952', 'dm_length_actuators')
number_of_actuators = CONFIG_INI.getint("boston_kilo952", "number_of_actuators")
image = np.zeros((number_of_actuators_per_dimension, number_of_actuators_per_dimension))
image[np.unravel_index(index952, image.shape)] = dm_command[:number_of_actuators]
image = np.zeros(mask.shape)
image[mask] = dm_command[:number_of_actuators]
Comment on lines -265 to +287
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Good reduction! It took me way too long to even figure out what the old code was doing.


return image


def convert_dm_image_to_command(dm_image, path_to_save=None):
# Flatten the gain_map using index952
mask = catkit.util.get_dm_mask()
index952 = np.flatnonzero(mask)

# Parse using index952.
image_1d = np.ndarray.flatten(dm_image)
dm_command = image_1d[index952]
# Take actuators corresponding to the DM mask
dm_command = dm_image[catkit.util.get_dm_mask().astype('bool')]

# Write new image as fits file
if path_to_save is not None:
Expand Down
10 changes: 6 additions & 4 deletions catkit/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ def find_package_location(package='catkit'):
return importlib.util.find_spec(package).submodule_search_locations[0]


def find_repo_location(package='cakit'):
def find_repo_location(package='catkit'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, thanks. 👍

return os.path.abspath(os.path.join(find_package_location(package), os.pardir))


def get_dm_mask():
mask_path = os.path.join(find_package_location("catkit"), "hardware", "boston", "kiloCdm_2Dmask.fits")
mask = fits.open(mask_path)[0].data
return mask
if not hasattr(get_dm_mask, 'mask'):
mask_path = os.path.join(find_package_location("catkit"), "hardware", "boston", "kiloCdm_2Dmask.fits")
get_dm_mask.mask = fits.getdata(mask_path)

return get_dm_mask.mask


# Does numpy gotchu?
Expand Down