Skip to content

[NMX] Allow setting output time bin unit.#545

Merged
YooSunYoung merged 4 commits into
mainfrom
nmx-output-t-unit
May 11, 2026
Merged

[NMX] Allow setting output time bin unit.#545
YooSunYoung merged 4 commits into
mainfrom
nmx-output-t-unit

Conversation

@YooSunYoung
Copy link
Copy Markdown
Member

I added output configuration to manually set output time bin unit.

At first, I thought it should be in the output configuration but then i realized it may be misleading when someone wants to use save_results function separately... So I just added it to workflow configuration.

@YooSunYoung YooSunYoung changed the title Allow setting output time bin unit. [NMX] Allow setting output time bin unit. Apr 27, 2026
@YooSunYoung YooSunYoung added the essnmx Issues for essnmx. label Apr 27, 2026
@github-project-automation github-project-automation Bot moved this to In progress in Development Board Apr 27, 2026
@YooSunYoung YooSunYoung moved this from In progress to Selected in Development Board Apr 27, 2026

# Histogram detector counts
output_tunit = config.workflow.result_time_bin_unit
t_bin_edges = t_bin_edges.to(unit=output_tunit)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If DIALS expects ns (the default), does it also expect integers?
I.e. if the output unit is ns, do we then need to convert also to int?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure... but it computes wavelength from the time again so shouldn't it prefer float...?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I asked Aaron in person but for now we want to keep the dtypes the same as the older version, which is the float64.

Copy link
Copy Markdown
Contributor

@jokasimr jokasimr left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment thread packages/essnmx/src/ess/nmx/executables.py
@YooSunYoung YooSunYoung enabled auto-merge May 11, 2026 14:11
Comment thread packages/essnmx/src/ess/nmx/executables.py Outdated
@YooSunYoung YooSunYoung added this pull request to the merge queue May 11, 2026
Merged via the queue into main with commit 6ecec6e May 11, 2026
5 checks passed
@YooSunYoung YooSunYoung deleted the nmx-output-t-unit branch May 11, 2026 14:32
@github-project-automation github-project-automation Bot moved this from Selected to Done in Development Board May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essnmx Issues for essnmx.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants