Skip to content

Conversation

@jokasimr
Copy link
Contributor

@jokasimr jokasimr commented Dec 2, 2025

Fixes #183

Will be rebased on main once #185 is merged.

@jokasimr jokasimr marked this pull request as ready for review December 11, 2025 09:04
@jokasimr jokasimr requested a review from nvaytet December 11, 2025 09:04
return sc.atan2(y=p.fields.x, x=p.fields.z) - detector_rotation.to(unit='rad')


def detector_ltotal_from_raw(
Copy link
Member

Choose a reason for hiding this comment

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

Can you remind me why we need a special provider for this for Estia?
I would have expected that the generic nexus workflow can compute Ltotal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no special reason. I didn't know there was a provider for that already. That's good 👍 I'll remove it and see if it still runs.



def mcstas_wavelength_coordinate_transformation_graph() -> CoordTransformationGraph:
def mcstas_wavelength_coordinate_transformation_graph(
Copy link
Member

Choose a reason for hiding this comment

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

May I suggest we move all the McStas-related stuff into the mcstas.py file?
I feel it would make things better organised.

proton_current: ProtonCurrent[RunType],
graph: CoordTransformationGraph,
graph: CoordTransformationGraph[RunType],
) -> ReducibleData[RunType]:
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but this reminds me that ReducibleData should probably be renamed to CorrectedDetector? (according to the new naming scheme)



def load_mcstas_events(
def load_mcstas_provider(
Copy link
Member

Choose a reason for hiding this comment

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

Can we also move the mcstas-related code into the mcstas file?

"id": "5",
"metadata": {},
"source": [
"## Detector view from the created nexus file"
Copy link
Member

Choose a reason for hiding this comment

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

I would keep the notebook concise and just have the conversion to nexus file and drop the detector view and computing reflectivity curves. That way, the notebook has a single clear goal.

@jokasimr jokasimr requested a review from nvaytet December 12, 2025 10:36
@jokasimr jokasimr mentioned this pull request Dec 12, 2025
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

Successfully merging this pull request may close these issues.

Nexus file with McStas data

3 participants