Skip to content

Run tutorials and tests in CI#35

Merged
mgt16-LANL merged 31 commits intolanl:Secondary_Solvation_Shellfrom
orionarcher:ci-testing
Jan 22, 2026
Merged

Run tutorials and tests in CI#35
mgt16-LANL merged 31 commits intolanl:Secondary_Solvation_Shellfrom
orionarcher:ci-testing

Conversation

@orionarcher
Copy link
Collaborator

No description provided.

@mgt16-LANL mgt16-LANL self-requested a review January 20, 2026 23:30
@orionarcher
Copy link
Collaborator Author

@mgt16-LANL I force-pushed and reset the workflow approval :(

Please reapprove when you get the chance!

@mgt16-LANL
Copy link
Collaborator

@mgt16-LANL I force-pushed and reset the workflow approval :(

Please reapprove when you get the chance!

Done! @orionarcher !

@orionarcher
Copy link
Collaborator Author

@mgt16-LANL, hmm, this time I did not force-push but approvals are still required again.

You may have to modify something on the repo settings or on my permissions for me to be able to iterate on this testing PR.

Any solution would be appreciated!

@mgt16-LANL
Copy link
Collaborator

mgt16-LANL commented Jan 21, 2026

@orionarcher - Let's fully depricate the xtb-python package. I wrote the CLI ASE class as bypass and don't want the headache of trying to continue to interface with an unsupported package (I'll take care of this). If the cli class is throwing errors I should update that ASE class. Also - is torch-dftd required for the base architector package? I'd prefer keeping non-required MLIP packages separate from the architector requirements.

@orionarcher
Copy link
Collaborator Author

orionarcher commented Jan 21, 2026

@mgt16-LANL - xtb-python seemed to be required for tutorials 1, 4, 5, 8, 9, and 13 to pass, they were failing before I added it into the install. What's the best way to get those passing without xtb-python? I don't know the package well enough to have a great alternative.

torch-dftd is only required for tutorial 14. The packages currently installed in the tutorials CI are the superset of all tutorials dependencies, rather than the base architector dependencies. Agreed these should be more clearly differentiated. I could add in a block such that torch-dftd it is only installed for that tutorial, or at the very least, I can indicate which tutorials the dependencies are required for.

The setup.py (or pyproject.toml) could also have optional dependency sets for different use cases.

@mgt16-LANL
Copy link
Collaborator

mgt16-LANL commented Jan 22, 2026

@orionarcher - do you know how to update the .github/workflows/deploy.yml to work with pyproject.toml? I'm a bit unfamiliar with the distribution. Still working on tracking down just why this is failing in CI without xtb-python when it works on all my different machines locally...

@orionarcher
Copy link
Collaborator Author

orionarcher commented Jan 22, 2026

@orionarcher - do you know how to update the .github/workflows/deploy.yml to work with pyproject.toml?

Yep, if it sounds good to you I am happy to swap the setup.py for a pyproject.toml, it's an increasingly popular/standard solution as I understand it.

Oop just saw your commit, let me take a look and see if I can get the release and everything else working with the pyproject.toml.

Still working on tracking down just why this is failing in CI without xtb-python when it works on all my different machines locally...

I can also do some investigating here. On my machine I think I installed xtb-python.

@mgt16-LANL
Copy link
Collaborator

@orionarcher Think i finally fixed all the issues by pinning xtb version higher and a few code tweaks. Happy to merge when the pypi deployment works :).

@orionarcher
Copy link
Collaborator Author

orionarcher commented Jan 22, 2026

@mgt16-LANL, done!

Last thing, are there are particular python versions that you are aiming to support? If so, we could iterate over those in testing.

If you didn't have something in mind, I'd advocate for python 3.11-3.13 since numpy recently dropped 3.10 support.

EDIT: taking a moment to clean up the dependencies in testing

Keep in conda (required):
- openbabel (C++ bindings)
- crest (CLI)
- xtb>6.5 (CLI)
- tblite-python (Fortran)
- jupytext (needed before pip install)

Install via pip from pyproject.toml:
- Core deps: ase, numpy, scipy, etc.
- Optional: mace, dftd, tutorials
@mgt16-LANL
Copy link
Collaborator

@mgt16-LANL, done!
Last thing, are there are particular python versions that you are aiming to support? If so, we could iterate over those in testing.
If you didn't have something in mind, I'd advocate for python 3.11-3.13 since numpy recently dropped 3.10 support.

I would love to support python 3.11-3.13!

The conda py3Dmol package depends on ipython, but pip py3Dmol doesn't.
Adding ipython to [tutorials] optional deps fixes the headless CI issue.

Conda (required):
- openbabel (C++ bindings)
- crest (CLI)
- xtb>6.5 (CLI)
- tblite-python (Fortran)
- jupytext (needed before pip install)

Pip via pyproject.toml:
- Core deps + [mace,dftd,tutorials]
@orionarcher
Copy link
Collaborator Author

Ok! I think this is now working. I just added 3.11 and 3.12 because 3.13 isn't supported by OpenBabel yet. Good to merge in my opinion!

@mgt16-LANL mgt16-LANL merged commit 7d6213b into lanl:Secondary_Solvation_Shell Jan 22, 2026
17 checks passed
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.

2 participants