Skip to content

Bump euphonic version, support .yml input to Abins #39881

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Bump euphonic version, support .yml input to Abins #39881

wants to merge 9 commits into from
+83 −25

Conversation

ajjackson
Copy link
Contributor

@ajjackson ajjackson commented Sep 1, 2025 •
edited
Loading

Description of work

janus-core is a package developed in STFC/SCD for convenient running of machine-learned interatomic potentials, including phonon calculations. However, its file-naming conventions are slightly incompatible with Abins expectations:

  • Abins expects .yaml extension for Phonopy data files, but janus-core uses .yml
  • Abins expects HDF5 force constants to be named force_constants.hdf5 in the same directory as the .yaml data, but janus-core typically saves them with some prefix prefix-force_constants.hdf5. This helps disambiguate multiple calculations in the same directory.

These cases are already handled in recent Euphonic releases. A little bit of logic is re-implemented for the Mantid algorithm validation stage.

To test:

The changes are covered by unit testing

Release notes have been added.

Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

  • Is the code of an acceptable quality? (Code standards/GUI standards)
  • Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
  • Are appropriately scoped unit and/or system tests provided?
  • Do the release notes conform to the guidelines and describe the changes appropriately?
  • Has the relevant (user and developer) documentation been added/updated?

Gatekeeper

As per the gatekeeping guidelines:

  • Has a thorough first line review been conducted, including functional testing?
  • At a high-level, is the code quality sufficient?
  • Are the base, milestone and labels correct?

@ajjackson ajjackson mentioned this pull request Sep 1, 2025
Merged
@ajjackson
Copy link
Contributor Author

CI looking happy again, I just need to generate some data for a unit test and this can be opened for review.

@sf1919 sf1919 added ISIS: Spectroscopy Issue and pull requests relating to Muons, Indirect and Inelastic at ISIS ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions labels Sep 2, 2025
@ajjackson ajjackson marked this pull request as ready for review September 3, 2025 11:21
@sf1919 sf1919 added this to the Release 6.14 milestone Sep 3, 2025
@peterfpeterson
Copy link
Member

Does this new version mean we can drop the build number pin for seekpath?

@ajjackson
Copy link
Contributor Author

No, there is no seekpath version since 2.1.0 so no way to bump to a half-open version requirement. The "hard pin" to the fixed build should stay at Mantid level for now.

@ajjackson
Copy link
Contributor Author

Hmm, that build failure is plausibly related to conda packaging but there doesn't seem to be anything useful in the log about what is wrong.

@ajjackson
Copy link
Contributor Author

That's interesting, I just got an email about a failed merge-conflicts run, linking to this page: https://github.com/ajjackson/mantid/actions/runs/17496436559/job/49698354937

But over here everything looks fine. Some quirk of how CI is set up for external fork contributions?

ajjackson and others added 9 commits September 9, 2025 15:23
@ajjackson
Bump Euphonic requirement to 1.4.5
f12b39c 
- This includes a bugfix to import of phonopy data with dipole-dipole
  contributions when using a non-symmetric supercell matrix

- This also provides support in the utilities for import of yaml files
  from janus-core, which differ slightly from Phonopy conventions.
@ajjackson
Support janus-core.yml files as Abins input
4486839 
If the filename follows pattern SEEDNAME-phonopy.yml, look for a
force constants file named SEEDNAME-force_constants.hdf5
@pre-commit-ci @ajjackson
[pre-commit.ci] auto fixes from pre-commit.com hooks
457b93c 
for more information, see https://pre-commit.ci
@ajjackson
Add release notes
85dc520
@ajjackson
Fix .yml phonon file validation logic
3860ba0 
- used incorrect method isfile()
- if instead of elif: led to spurious complaint about missing force constants
@ajjackson
Refactor FORCECONSTANTS validation logic, add unit tests
602748f
@ajjackson
Typo in systemtest docstring
49c49cb
@ajjackson
Add note on janus-core to Abins Data Formats docs.
1de5d87
@ajjackson
Show phonopy .yml extension option in Abins(2D) algorithm docs
8f5cbcd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions ISIS: Spectroscopy Issue and pull requests relating to Muons, Indirect and Inelastic at ISIS
Projects
ISIS core workstream v6.14.0
Status: In review
Milestone
Release 6.14
Development

Successfully merging this pull request may close these issues.
3 participants
@ajjackson @peterfpeterson @sf1919