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

Device for lakeshores temperature controllers #493

Closed
gfabbris opened this issue Jan 21, 2021 · 16 comments · Fixed by #603
Closed

Device for lakeshores temperature controllers #493

gfabbris opened this issue Jan 21, 2021 · 16 comments · Fixed by #603
Assignees
Milestone

Comments

@gfabbris
Copy link
Collaborator

I believe most beamlines from the MM and IXN groups use the lakeshore 340. At 4-ID-D we also have the 336. The 4ID-POLAR setup has the setup for both (340, 336), but it could probably be tweaked for generality.

@prjemian
Copy link
Contributor

There are several lakeshore controllers in the ip module. Can these be generalized? Time to examine the .db files:

.db file last updated
LakeShore330.db 2005-12-05
LakeShore331.db 2009-06-03
LakeShore335.db 2018-04-09
LakeShore336.db 2018-04-09
LakeShore340.db 2007-06-18
LakeShoreDRC-93CA.db 2004-09-24

@prjemian
Copy link
Contributor

prjemian commented Oct 15, 2021

beam line ophyd support
APS Polar 4-ID-D LakeShore 340
APS Polar 4-ID-D LakeShore 336
APS XPCS 8_ID-I LakeShore 336

@prjemian
Copy link
Contributor

@gfabbris , @qzhang - There are some differences between XPCS and Polar implementations of the LakeShore 336 support. At Polar, loop1 is for control and loop 2 is for the sample, loops 3 & 4 are read-only.

class LS336Device(Device):
    loop1 = FormattedComponent(LS336_LoopControl, "{prefix}", loop_number=1)
    loop2 = FormattedComponent(LoopSample, "{prefix}", loop_number=2)
    loop3 = FormattedComponent(LS336_LoopRO, "{prefix}", loop_number=3)
    loop4 = FormattedComponent(LS336_LoopRO, "{prefix}", loop_number=4)

At XPCS, it's a little less detailed:

class LS336Device(Device):
    loop1 = FormattedComponent(LS336_LoopMore, "{self.prefix}", loop_number=1)
    loop2 = FormattedComponent(LS336_LoopMore, "{self.prefix}", loop_number=2)
    loop3 = FormattedComponent(LS336_LoopBase, "{self.prefix}", loop_number=3)
    loop4 = FormattedComponent(LS336_LoopBase, "{self.prefix}", loop_number=4)

Any advice how to proceed? Is each LakeShore 336 the same or does the user choose what to do with each loop?

@prjemian
Copy link
Contributor

A bigger difference that I see is that the 8-ID-I XPCS implementation (LS336_LoopBase()) allows loops 3 & 4 to be control loops but 4-ID-C Polar (LS336_LoopRO()) treats them as read-only:

class LS336_LoopRO(Device): # Polar
    """
    loop3 and loop4 do not use the heaters.
    """
    readback = FormattedComponent(EpicsSignalRO, "{prefix}IN{loop_number}", kind="normal")


class LS336_LoopBase(ProcessController):  XPCS
    """
    One control loop on the LS336 temperature controller
    
    Each control loop is a separate process controller.
    """
    signal = FormattedComponent(EpicsSignalRO, "{self.prefix}OUT{self.loop_number}:SP_RBV")
    target = FormattedComponent(EpicsSignal, "{self.prefix}OUT{self.loop_number}:SP", kind="omitted")

(NOTE: These will be refactored into apstools.devices.PVPositionerSoftDoneWithStop.)

@prjemian
Copy link
Contributor

prjemian commented Oct 15, 2021

In the end, should we have one LakeShore336Device() which all can use or does each implementation need to be built from parts (like an area detector support)?

@prjemian
Copy link
Contributor

@gfabbris - Vaporizer - is that specific to Polar?

@gfabbris
Copy link
Collaborator Author

The lakeshore336 support at Polar is too specific for how we use it. For instance, we actually want to control only loop2 (sample temperature), and have loop1 track it (vaporizer - this is where the LHe comes into the sample space). Looking at it right now, it is also too complicated, I need to clean it up at some point.

But your previous question is a very good one. I'm inclined towards having a single support, where we assume that all loops are controlling a heater. I think it would then be easy for a particular beamline to just ignore the extra loops (or, preferably, just set them to None). For example, at Polar, it doesn't make sense to have loop3 and 4 there, it's just storing useless baseline data.

prjemian added a commit that referenced this issue Oct 15, 2021
@prjemian
Copy link
Contributor

@gfabbris The 340 support looks similarly complicated. Comment?

@prjemian
Copy link
Contributor

I'm missing the temperature Component attribute (subclass from ophyd.PVPositioner) that users will use to control the temperature. Expecting to see user plans that use yield from bps.mv(ls336.sample.temperature, 720) or similar. This is self-documenting code and very intuitive.

This LakeShore support is not designed that way now (as above: yield from bps.mv(ls336.loop2, 720) # set sample temperature) and needs to be refactored. The entire device is a controller (with ramp, heater, enable, PID, and other capabilities). It needs a temperature attribute. For the 336, there are 4 of them, one in each loop. The 340 has two of them.

Other temperature controllers in apstools:

(bluesky_2021_2) prjemian@zap:~/.../BCDA-APS/apstools$ git grep temperature | grep Component
apstools/_devices/linkam_controllers.py:    temperature = Component(
apstools/_devices/linkam_controllers.py:    temperature_in = Component(EpicsSignalRO, "tempIn", kind="omitted")
apstools/_devices/linkam_controllers.py:    # DO NOT USE: temperature2_in = Component(EpicsSignalRO, "temp2In", kind="omitted")
apstools/_devices/linkam_controllers.py:    # DO NOT USE: temperature2 = Component(EpicsSignalRO, "temp2")
apstools/_devices/linkam_controllers.py:    temperature = Component(
apstools/_devices/ptc10_controller.py:        readback = Component(EpicsSignalRO, "2A:temperature", kind="hinted")
apstools/_devices/ptc10_controller.py:    temperature = Component(EpicsSignalRO, "temperature", kind="normal")
apstools/_devices/ptc10_controller.py:    temperature = Component(EpicsSignalRO, "temperature", kind="normal")

@prjemian
Copy link
Contributor

Looks as if this will take more time to resolve and could miss the planned deadline to release 1.5.3 by the end of this month. There is a Eurotherm 2216e controller, #559, in that milestone as well. Pushing to the next milestone.

@prjemian prjemian modified the milestones: 1.5.3, 1.6.0 Oct 15, 2021
prjemian added a commit that referenced this issue Oct 15, 2021
@prjemian prjemian self-assigned this Oct 15, 2021
@prjemian
Copy link
Contributor

Summary, each loop should not be a positioner. Instead, each loop should have a temperature attribute that is a positioner, to provide consistency across all the temperature controllers to be supported.

Other controller features (such as pressure, vacuum, ...) could also be positioners if the controller supports such.

@prjemian
Copy link
Contributor

Control screens

The control screens depict the fields most useful to end users:

LS336 more

ls336_more

LS340 loop1 more

ls340_more

@prjemian
Copy link
Contributor

prjemian commented Dec 1, 2021

@gfabbris Since this branch was last edited, there have been significant changes in the apstools repository, preparing for the 1.6.0 release.

It's time to merge those changes (from the main branch) into this branch before continuing.

@prjemian
Copy link
Contributor

prjemian commented Dec 5, 2021

@gfabbris

Commit 545a992 belongs here. It is a new branch, compatible with the refactor for release 1.6, that copies the lakeshore_controllers.py file from the original branch.

@prjemian
Copy link
Contributor

prjemian commented Dec 5, 2021

@gfabbris If you agree the new branch (493-v2-lakeshore) covers the work-in-progress now, you can delete the original 493-lakeshore branch on the branches page.

@gfabbris
Copy link
Collaborator Author

gfabbris commented Dec 7, 2021

Looks good, deleting the original branch.

prjemian added a commit that referenced this issue Jan 20, 2022
prjemian added a commit that referenced this issue Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants