logsdail / carmm

Scripts for creation, manipulation and analysis of geometric and electronic structure of molecular models
GNU General Public License v3.0
5 stars 18 forks source link

Adds 'custom' HPC option to React workflow. #143

Closed GabrielBram closed 7 months ago

GabrielBram commented 7 months ago

Some of the solutions are a bit hacky here, but I am working on the assumption that only users with some experience of setting up their AIMs commands would be interested in this. This allows flexibility to set custom FHI-aims commands, meaning React can be run on desktops/alternative compilations without changing hardcoded values.

Needing a new environmental variable for the FHI-aims root directory is also less than desirable, but intended to avoid: A) expecting a key word on the React level, which must be passed down all the way to AIMs path, B) problems with the AIMS_SPECIES_DIRECTORY, which would not work with the existing logic for selecting basis levels.

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 94.64286% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 88.03%. Comparing base (6c6e942) to head (8114df1). Report is 2 commits behind head on main.

Files Patch % Lines
carmm/run/aims_path.py 70.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #143 +/- ## ========================================== + Coverage 87.90% 88.03% +0.13% ========================================== Files 76 78 +2 Lines 3149 3202 +53 ========================================== + Hits 2768 2819 +51 - Misses 381 383 +2 ``` | [Flag](https://app.codecov.io/gh/logsdail/carmm/pull/143/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Logsdail) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/logsdail/carmm/pull/143/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Logsdail) | `88.03% <94.64%> (+0.13%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Logsdail#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ikowalec commented 7 months ago

Nothing wrong with this, but given that the way the handling of the FHI-aims species directory and command of Aims calculator are handled in ASE 3.23, we might want to think about future-proofing the method. My recommendation is to leave that as a TODO prior to merge.