Skip to content
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.

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

Update the szczur environment file to match current environment variable syntax #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mgduda
Copy link
Owner

@mgduda mgduda commented Mar 6, 2020

This merge updates the szczur.yaml environment file to match the current environment
variable syntax.

As of the merge commit 6f55178, the syntax for specifying environment variables
changed.

@mgduda
Copy link
Owner Author

mgduda commented Mar 6, 2020

This PR fixes Issue #15 .

@mgduda mgduda requested a review from MiCurry March 6, 2020 21:01
Copy link
Collaborator

@MiCurry MiCurry left a comment

Choose a reason for hiding this comment

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

@MGuda, the library section looks good in szczur.yaml, but the MPI section is incorrect. You can specify the MPI section in a similar way to the compiler section:

  GNU-9.2.0:
    name: gnu-9.2.0
    compiler:
      name: gnu
      version: 9.2.0
      path: /Users/duda/local/gcc-9.2.0
      executables:
        - gcc
        - gfortran
        - g++
    MPI: 
      version: x.xx.x
      PATH: /Users/duda/local/gcc-9.2.0
      executables:
        - mpicc
        - mpifort
        - mpic++

@MiCurry
Copy link
Collaborator

MiCurry commented Mar 9, 2020

I will also note that I've got a hopefully final change to the main specification of the environment file coming soon, which you can find in my branch here: https://github.com/MiCurry/SMARTS2/tree/env_spec. As well as updating the specification to be more consistent, it also contains a few more checks to the environment file and updates all of the error messages to match the specification.

If you would like, I can update szczur.env to match the new environment in that branch after merging this PR.

…syntax

As of the merge commit 6f55178, the syntax for specifying environment variables
changed. This commit updates the szczur.yaml file to match this new syntax.
@mgduda
Copy link
Owner Author

mgduda commented Mar 10, 2020

I've just force-pushed an updated branch that I think should resolve the MPI environment issue.

@MiCurry
Copy link
Collaborator

MiCurry commented Apr 8, 2020

@mgduda, this PR looks good. However, after the last PR #17 the file is again out of date. However, PR #17 contains update to the environment file parser that will walk you through the changes you'll need to make. So if you rebase this branch on master, and try to run list tests you'll get error messages on what needs to be change.

The changes are mostly for the capitalization of keywords (i.e. compiler becomes Compiler, name becomes Name, etc.)

Let me know if you have any questions or problems.

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