Conversation
kfir4444
reviewed
Jun 18, 2026
| # 4) HuggingFace authentication for the gated uma-s-1p1 model. | ||
| if [ "$SKIP_HF_LOGIN" -eq 0 ]; then | ||
| if $COMMAND_PKG run -n "$ENV_NAME" huggingface-cli whoami &>/dev/null; then | ||
| echo "✔️ Already authenticated to HuggingFace." |
Collaborator
There was a problem hiding this comment.
The workflow is not clear to me. Are we not able to utilize the HF_TOKEN if the user has it, or parse it into the script?
| PYCODE | ||
|
|
||
| # 4) HuggingFace authentication for the gated uma-s-1p1 model. | ||
| if [ "$SKIP_HF_LOGIN" -eq 0 ]; then |
Collaborator
There was a problem hiding this comment.
SKIP_HF_LOGIN is potentially confusing, are we skipping it for testing or since the user is logged in?
| - setuptools | ||
| - pip | ||
| - pip: | ||
| # fairchem-core pulls in a CUDA-enabled PyTorch by default on Linux. |
Collaborator
There was a problem hiding this comment.
We should probably let them know that. Also, can we allow users to choose which device they are using? like uma-cpu and uma-gpu?
| return MOPAC(**kwargs) | ||
|
|
||
|
|
||
| elif name in ('uma', 'fairchem'): |
Collaborator
There was a problem hiding this comment.
Does fairchem has a model that is not uma? Do we want to leave the option to call fairchem in the input?
| # A TS search needs a saddle-point optimizer; UMA ships none, so use Sella. | ||
| from sella import Sella | ||
| opt_class = Sella | ||
| opt = Sella(atoms, order=1 if is_ts else 0, logfile=logfile) |
Collaborator
There was a problem hiding this comment.
We use sella when is_ts == True, why does order != 1 always?
| save_current_geometry(output, atoms, xyz) | ||
|
|
||
| if job_type == 'irc': | ||
| from sella import IRC # VERIFY the Sella IRC API in the installed sella |
Collaborator
There was a problem hiding this comment.
Can you drop the comment? did we verify the IRC API?
| opt_class = engine_dict.get(engine_name, BFGS) | ||
| opt = opt_class(atoms, logfile=os.path.join(os.path.dirname(input_path), 'opt.log')) | ||
|
|
||
| opt = engine_dict.get(engine_name, BFGS)(atoms, logfile=logfile) |
Collaborator
There was a problem hiding this comment.
I think the original implementation was more readable.
| if is_atom or is_triplet_o2: | ||
| label = self.species[0].label if self.species else 'species' | ||
| logger.warning(f'Computing a UMA single point for {label} (an isolated atom or triplet O2). ' | ||
| f'UMA absolute energies are unreliable for these under-represented species; ' |
Collaborator
There was a problem hiding this comment.
Can you add a source for that please? Is that like a known issue?
| """ | ||
| scan_coords = self.data.get('scan_coords') | ||
| if scan_coords: | ||
| return [xyz if isinstance(xyz, dict) else str_to_xyz(xyz) for xyz in scan_coords] |
Collaborator
There was a problem hiding this comment.
Amazing! I missed that we didn't knew how to interact with scans from ase. Can you please add tests to these functions too?
kfir4444
force-pushed
the
uma
branch
4 times, most recently
from
b628acc to
2549a2c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #895 +/- ##
==========================================
+ Coverage 63.02% 63.14% +0.11%
==========================================
Files 114 114
Lines 38178 38218 +40
Branches 9990 9999 +9
==========================================
+ Hits 24063 24132 +69
+ Misses 11227 11194 -33
- Partials 2888 2892 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add devtools/uma_environment.yml and devtools/install_uma.sh: a documented, user-driven setup script that wraps every step (create uma_env, verify fairchem/Sella/ASE imports, interactive HuggingFace login for the gated model, and the runtime exports for invoking UMA from arc_env). Supports --test to run the model-dependent unit tests and --skip-hf-login. Wire 'make install-uma' and .PHONY, and document UMA setup in installation.rst. Deliberately NOT added to devtools/install_all.sh / install-ci: the UMA model is gated (Meta license + HuggingFace token) and heavy, so it is a manual setup; CI coverage comes from the env-independent UMA tests.
Build UMA (Meta FAIR fairchem-core, uma-s-1p1, task omol) on top of the generic ASE adapter (PR #836) instead of a standalone adapter: - ase_script.py: add a uma/fairchem branch to get_calculator; set total charge and spin (=multiplicity) on atoms.info (omol conditioning); use Sella order=1 for TS saddle-point searches when is_ts; add an irc job type via Sella IRC. - ase.py: derive the calculator from the level method (so method='uma' works with no args), resolve UMA defaults (latest model, omol, cpu) via determine_settings, pass is_ts and irc_direction to the script, and warn on a UMA single point for an isolated atom or triplet O2 (unreliable absolute energy). - settings.py: UMA_PYTHON=find_executable('uma_env'), ASE_CALCULATORS_ENV['uma'], and UMA_LATEST_MODEL. - level.py: route method 'uma'/'uma-s-1'/'uma-s-1p1' to the 'ase' software. - yaml.py: implement parse_irc_traj and parse_1d_scan_coords so UMA IRC/scan outputs round-trip. Rotor scans run through ARC's directed_scan (constrained opt), already supported by the ASE adapter. fairchem/Sella-IRC API points only confirmable inside uma_env are marked with # VERIFY. Adds env-independent unit tests (routing, calculator/settings resolution, input writing, sp warning, output round-trip) plus skip-guarded model tests.
Made `warn_if_unreliable_uma_sp` a bolean function, check the return value. Doesn't interfere with current implementation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently sits on top of #836