Skip to content

Tofectomy#207

Merged
jokasimr merged 15 commits intomainfrom
tofectomy
Mar 27, 2026
Merged

Tofectomy#207
jokasimr merged 15 commits intomainfrom
tofectomy

Conversation

@jokasimr
Copy link
Copy Markdown
Contributor

@jokasimr jokasimr commented Mar 18, 2026

Needs a new release of essreduce before being merged.

@jokasimr jokasimr requested a review from nvaytet March 18, 2026 13:59
Copy link
Copy Markdown
Member

@nvaytet nvaytet 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 👍
Let's see what the CI says once we release essreduce.

@jokasimr jokasimr requested a review from nvaytet March 24, 2026 15:15
@nvaytet
Copy link
Copy Markdown
Member

nvaytet commented Mar 24, 2026

Changes look ok, but there seems to be something up with the user-guide/estia/simulated-spin-flip-sample.ipynb notebook?

Copy link
Copy Markdown
Member

@nvaytet nvaytet left a comment

Choose a reason for hiding this comment

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

@jokasimr said he would make new files for running tests/docs.

(just requesting changes so this doesn't doesn't get merged accidentally)

proton_current=proton_current,
graph={
**graph,
"wavelength": lambda wavelength_from_mcstas: wavelength_from_mcstas,
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.

👍


return batch.compute(target)
result = batch.compute(target)
return result if runs_is_mapping else list(result.values())
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.

Do we want to do this, or do we always want to return a mapping? (which is easier to work with I think?)

We could above just convert runs to a dict if it is not a mapping, and leave the rest un-touched?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The type hints of the function says that it accepts either a sequence or a mapping, but currently if you pass a sequence it errors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think in some cases it can be more convenient to just pass a sequence and get a sequence back 🤷

Co-authored-by: Neil Vaytet <39047984+nvaytet@users.noreply.github.com>
@jokasimr jokasimr merged commit 3267486 into main Mar 27, 2026
4 checks passed
@jokasimr jokasimr deleted the tofectomy branch March 27, 2026 10:59
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.

3 participants