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

Improvements to the goniometer/detector definition #57

Open
DominicOram opened this issue Jul 7, 2022 · 9 comments
Open

Improvements to the goniometer/detector definition #57

DominicOram opened this issue Jul 7, 2022 · 9 comments

Comments

@DominicOram
Copy link
Collaborator

DominicOram commented Jul 7, 2022

As a user of nexgen the interface around call_writers and the goniometer and detector is a bit confusing. Particularly using dictionaries leaves a lot of room for error.

Acceptance Criteria

  • The code for defining goniometers and detectors is refactored to be less error prone
  • There are unit tests covering all of this refactor
@DominicOram
Copy link
Collaborator Author

DominicOram commented Aug 9, 2022

As an example I would suggest something like:

@dataclass
class AxisDefinition():
    name: str
    depends_on: Optional[AxisDefinition]
    vector: Point3D
    type: MovementTypeEnum
    unit: pint.unit.Unit
    offset: float
    initial_position: float

@dataclass
class Goniometer():
    axes: List[AxisDefinition]
    
    def __post_init__():
        #Some validation here that geometry actually makes sense

@DominicOram
Copy link
Collaborator Author

More than happy to discuss other ideas though

@rjgildea
Copy link
Contributor

rjgildea commented Aug 9, 2022

As an example I would suggest something like:

@dataclass
class AxisDefinition():
    name: str
    depends_on: Optional[AxisDefinition]
    vector: Point3D
    type: MovementTypeEnum
    unit: pint.unit.Unit
    offset: float
    initial_position: float

@dataclass
class Goniometer():
    axes: List[AxisDefinition]
    
    def __post_init__():
        #Some validation here that geometry actually makes sense
        
def write_file(gonio: Goniometer, scan: scanspec.Spec, ...)
      # Note scan should only contain axes that are moving, otherwise AxisDefinition.initial_position assumed constant

I recall @ndevenish had an interface for writing NeXus files based around pydantic which would give you a dataclass-like interface with additional automatic type validation and conversion.

@ndevenish
Copy link

That was this thing. I've had some chats with Noemi and I've promised to put in an initial PR to start fleshing something in the next week or two.

@ndevenish
Copy link

It's somewhat a balance between conciseness and the complete generality of the NX definitions, but I think it can be made to align with both.

@DominicOram
Copy link
Collaborator Author

Thanks @ndevenish. I think something like that and using scanspec to define what the actual scans are is what I'm thinking.

@ndevenish
Copy link

scanspec

Is?

@graeme-winter
Copy link
Contributor

scanspec

Is?

https://github.com/dls-controls/scanspec

Is this?

@DominicOram
Copy link
Collaborator Author

Sorry, yes, that. I'm not wedded to it particularly but it seems like a neat common interface to describe a scan and keeping the maths of e.g. what a gridscan looks like in one place

@DominicOram DominicOram changed the title Discuss: Improvements to the goniometer definition / scan definition interface Discuss: Improvements to the goniometer definition Jan 20, 2023
@DominicOram DominicOram changed the title Discuss: Improvements to the goniometer definition Discuss: Improvements to the goniometer/detector definition Jan 20, 2023
@DominicOram DominicOram changed the title Discuss: Improvements to the goniometer/detector definition Improvements to the goniometer/detector definition Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants