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.

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

Refactor BostonDM to only have controlled actuator on data streams #123

Open
ehpor opened this issue Oct 13, 2023 · 7 comments
Open

Refactor BostonDM to only have controlled actuator on data streams #123

ehpor opened this issue Oct 13, 2023 · 7 comments
Labels
refactor Refactor existing code.

Comments

@ehpor
Copy link
Collaborator

ehpor commented Oct 13, 2023

This would remove one of three formats for DM commands. All current DM command formats are:

  1. The BMC command (shape for HiCAT is 2048).
  2. The active/controlled actuators command (shape for HiCAT is 1904). (edit by @ivalaginja, see Add DM base class. #156 (comment))
  3. The DM maps (shape for HiCAT is 2x 34x34).

After this change, only the last two would remain. This would make it simpler to distinguish between them, since one of them is 2D and the other is 1D. We use format 2 extensively for WFC and format 3 for plotting/display. However, format 1 is only used for communicating with the DM. This could be internalized inside the BMC DM service, being invisible to the outside. The conversion between active/controlled actuators and BMC command can be done purely based on data. This would also make it easier for the THD2 team to use our exact service for their BMC DMs.

@ehpor ehpor added the refactor Refactor existing code. label Oct 13, 2023
@ehpor
Copy link
Collaborator Author

ehpor commented Oct 13, 2023

To formalize nomenclature, we could rename format 2 "commands" and format 3 "maps".

Additionally, we can make the DM maps 3D, with the active/controlled actuators for each map (defined by the dm_mask, also 3D) being concatenated to form the command. This would make the proxy fully data-driven as well.

@ehpor
Copy link
Collaborator Author

ehpor commented Oct 13, 2023

@ivalaginja Since THD2 has internal functions to convert DM maps to surface figures, I presume based on measured influence functions / calibration, would THD2 consider moving this functionality to the BMC DM proxy?

@ivalaginja
Copy link
Collaborator

ivalaginja commented Oct 14, 2023

@ehpor yes to any changes that generalize the DM proxy and sending commands to the DMs. I def want to move more things into catkit2 wherever possible, especially if it means that we do not have to write a completely new DM service for THD2. However, I have to admit you lost me a little bit in your explanations - I list a couple of questions below this paragraph to make sure I understand what exactly you are aiming at.

Could you confirm that the below statements are correct?

  1. The main goal of this change is to hide format 1 from the proxy and have it be used exclusively in the service. This means changing the total_voltage and total_surface data streams to carry commands of format 2.
  2. This also means that all hard-coded numbers used for the conversions between the command formats will be either read from the config or/and from properties (e.g., num_actuators_dm, the config entry and/or the property).
  3. When you say: "The conversion between active actuators and BMC command can be done purely based on data", with "data" you refer to the DM maps and the values I mention in statement 2.
  4. By extending format 3 by one dimension, an arbitrary number of DM maps (for format 3) can be used in the proxy, which generalizes the BMC service and proxy so that it does not require the use of a pair of equal DMs anymore - making it possible for us at THD2 to use the same service (and proxy).
  5. Irrelevant for now, but statement 4 also means the service gets generalized to an arbitrary number of DMs, is that correct?

And a question:
6. Which internal functions of THD2 are you referring to? As stated above I'd totally move stuff to the proxy I am just unsure right now what you mean.

@ehpor
Copy link
Collaborator Author

ehpor commented Oct 16, 2023

@ivalaginja Answers to your questions:

  1. Yes, and also the data on all of the channels, which are currently in format 1.
  2. Yes, rather than hardcoded in some of the functions in the proxy, as they are now.
  3. Yes, only config and fits files linked to from the config.
  4. Exactly. And I think in most cases, the format will be similar for us. It's mostly a change to using a tuple instead of two arguments in the BostonProxy.apply_shape() function, which is trivial to convert.
  5. Yes, dependent on the shape of the dm_mask fits file, and likely a separate value in the config, that will be checked with the shape of the dm_mask fits file upon reading from disk.
  6. I remember Pierre converting DM maps (ie. voltages) to a surface map (ie. completely smooth, many more pixels) for me. I think that was IDL code. It was a separate operation, done manually at the time. I only got surface maps for some of the DM maps that we used. I got a somewhat long lecture on the importance of surface maps vs. DM maps and how they are different from Pierre or Raphael, but I don't remember the details (this was 3.5 years ago now).

@ivalaginja
Copy link
Collaborator

ivalaginja commented Oct 16, 2023

Thanks a lot @ehpor! Sounds like a good way forward. Would you like me to get started on this?

I still don't know exactly what kind of drivers we have - the current infrastructure sends separate commands for our two DMs to the dll, which communicates with two separate cards. It sounds like it's just a matter of identifying the right way to send commands, we are currently looking into it.

Also, would you remind me what the latest status is on #8 #85? I wonder what it would mean for us and whether we need to anticipate problems, one way or another (likely yes, of some form).

As for point 6., ah yes, I see what you mean. Raphael G and I literally talked about this today. We indeed have this (the lecture is still outstanding for me lol), but this is not used in the hardware controls. If you still think this would be useful to have in the proxy, I can certainly have a look at converting the relevant IDL code.

@ehpor
Copy link
Collaborator Author

ehpor commented Oct 16, 2023

@ivalaginja

Thanks a lot @ehpor! Sounds like a good way forward. Would you like me to get started on this?

I can give it a shot, moreso to flesh out some of the ideas that are only in my head atm. (Also related to extracting a DmService base class.)

I still don't know exactly what kind of drivers we have - the current infrastructure sends separate commands for our two DMs to the dll, which communicates with two separate cards. It sounds like it's just a matter of identifying the right way to send commands, we are currently looking into it.

As you know, we're using the Python API provided by BMC. I'm 100% certain you are not using that rn. But I'm unsure of what Raphael/Pierre/Remi are comfortable with, with switching.

Also, would you remind me what the latest status is on #8? I wonder what it would mean for us and whether we need to anticipate problems, one way or another (likely yes, of some form).

I think you linked the wrong issue/PR?

As for point 6., ah yes, I see what you mean. Raphael G and I literally talked about this today. We indeed have this (the lecture is still outstanding for me lol), but this is not used in the hardware controls. If you still think this would be useful to have in the proxy, I can certainly have a look at converting the relevant IDL code.

I have never seen the code, only the result, so I don't know if we can reproduce something with the data we have at STScI for our DM. It would be nice to have. As such, I don't know the fidelity of that model either. But since Johan and the multiconjugate AO group at Paris have extensive experience with DM calibration, I wouldn't be surprised if it's more sophisticated than simple influence functions / gain, but also if it requires calibration data that we have never taken. A separate issue in any case.

@ivalaginja
Copy link
Collaborator

@ehpor ok, let me know if there's anything I can help with!

Yes it was supposed to be #85 haha. We do indeed not use the Python API right now, we'll have to discuss internally whether we want to look into it.

A separate issue in any case yes, I will keep it on my radar.

@ivalaginja ivalaginja mentioned this issue Mar 25, 2024
4 tasks
@ivalaginja ivalaginja changed the title Refactor BostonDM to only have active actuators on data streams Refactor BostonDM to only have controlled actuators command on data streams Apr 23, 2024
@ivalaginja ivalaginja changed the title Refactor BostonDM to only have controlled actuators command on data streams Refactor BostonDM to only have controlled actuator on data streams Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor existing code.
Projects
None yet
Development

No branches or pull requests

2 participants