30 update codebase to work with Python 3.8 (and thus ase v3.23) #163

logsdail · 2024-06-03T09:10:35Z

Testing to updated codebase with ASE release

Update the yaml files so now we are testing ASE v3.23.0.

v3.23.0 requires Python 3.8, so making this update.

Backtracking, to test what behaviour we get with Python 3.7

codecov · 2024-06-03T09:28:44Z

Codecov Report

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

Project coverage is 89.38%. Comparing base (5e7334c) to head (68e195b).

Files	Patch %	Lines
carmm/run/aims_calculator.py	94.11%	1 Missing ⚠️
examples/run_aims.py	66.66%	1 Missing ⚠️
examples/utils_python_env_check.py	96.96%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
+ Coverage   89.10%   89.38%   +0.27%     
==========================================
  Files          81       82       +1     
  Lines        3286     3382      +96     
==========================================
+ Hits         2928     3023      +95     
- Misses        358      359       +1

Flag	Coverage Δ
unittests	`89.38% <97.93%> (+0.27%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Changed to version Python 3.8 - should we go higher?

logsdail · 2024-06-03T10:07:34Z

Noting here that the docs runs with Python 3.9 - should we standardise to this version, perhaps?

logsdail · 2024-06-05T17:31:01Z

So I think after discussion today at a Group meeting, this has become an "update Python version to the most suitable for our HPC architectures".

Current Python compatibility of code: Python 3.7 (i.e., CI tests work to this version)

Current HPC system Python provisions:
Hawk: 3.7 (used currently, default), 3.9.2, 3.10.4, 3.11.2
Isambard: 3.7.3.2 (used currently), 3.8.5.1 (default)
Young: 3.6 (used currently), 3.7.0, 3.7.2, 3.7.4, 3.8.0, 3.8.6 (default), 3.9.0, 3.9.1, 3.9.6, 3.9.10, 3.11.3, 3.11.4
ARCHER2: 3.6.15 (used currently) 3.9.13.1 (default) 3.10.10

logsdail · 2024-06-05T17:31:57Z

So I think after discussion today at a Group meeting, this has become an "update Python version to the most suitable for our HPC architectures".

Current Python compatibility of code: Python 3.7 (i.e., CI tests work to this version)

Current HPC system Python provisions: Hawk: 3.7 (used currently, default), 3.9.2, 3.10.4, 3.11.2 Isambard: 3.7.3.2 (used currently), 3.8.5.1 (default) Young: 3.6 (used currently), 3.7.0, 3.7.2, 3.7.4, 3.8.0, 3.8.6 (default), 3.9.0, 3.9.1, 3.9.6, 3.9.10, 3.11.3, 3.11.4 ARCHER2: 3.6.15 (used currently) 3.9.13.1 (default) 3.10.10

My take home from this is the minimum we should go to is 3.8 or 3.9, but certainly not towards the high numbers.

Remove some redundant comments.

Added new file to just do Python 3.8 linter checks.

Revert existing Linter to Python 3.7, and name appropriately (so can check and errors arising)

Updated CI tests to run for Python 3.7 explicit. Will duplicate for Python 3.8 and Python 3.9

Added Python 3.8 check

Realised I can just do this with a version matrix.

Realised I can do this with a version matrix.

Setup linter to run over multple Python versions. Probably overkill but good testbed

Upgrade the CI tests (also has an action version update)

@GaryLZW

…ims interface, and when the test fails it doesn't revert to the correct directory. Requires some careful thinking how to resolve @GaryLZW

… is true

…d of Python

* Accommodate ase 3.23 * clean up some print statements * Change assertion line due to change of input file style. Write input files for 3.23 case --------- Co-authored-by: Andrew Logsdail <LogsdailA@cardiff.ac.uk>

…:logsdail/carmm into 30-update-codebase-to-work-with-ase-v323

logsdail · 2024-08-01T07:25:59Z

@PavelStishenko as suggested I've put a tag on the mainline version, and once merged we'll bumpy the minor version number.

ikowalec

There is some tidy up to do, but that is not related to the compatibility issues, which have been resolved successfully.

carmm/utils/python_env_check.py

examples/analyse_counterpoise.py

examples/analyse_forces.py

.github/workflows/main.yml

examples/analyse_counterpoise.py

examples/build_symmetry_mirror.py

carmm/utils/python_env_check.py

carmm/run/workflows/react.py

ikowalec · 2024-08-01T09:19:25Z

I suggest a second review before merge.

logsdail · 2024-08-01T10:27:17Z

@ikowalec thanks for comments - all addressed. Does someone else want to do a second review, please?

…to change calculator type here depending on ASE version

logsdail · 2024-08-01T12:07:19Z

@GaryLZW I simplified the analyse_counterpoise test, as the checking for ASE now happens in get_aims_calculator. The CI tests pass but you might want to check if the code works correctly for further use cases.

carmm/run/aims_calculator.py

GaryLZW · 2024-08-02T11:38:49Z

@GaryLZW I simplified the analyse_counterpoise test, as the checking for ASE now happens in get_aims_calculator. The CI tests pass but you might want to check if the code works correctly for further use cases.

This looks fine to me^^

… the code version

FirasAssa · 2024-08-04T21:12:45Z

During my project the following changes were made:
The err_handler folder was removed and a utils folder was created in its place
A python_env_check file was created with python_env_check() and is_env_python() inside
Implemented python_env_check() in anylyse_forces example
Implemented python_env_check() in analyse_calculator
Restricted mace-torch to 0.3.4, because it was failing on v3.7
Updated CatLearn to the most recent version to fix import error in mlneb.py, by getting the version of CatLearn directly from Git

Thinks still to do after the project:
In python v3.8 ci-test there are 4 places where .BadConfiguration Error occurs. Presumably it is the same type of problem that can be fixed in one go
Errors:
ERROR analyse_counterpoise.py - ase.calculators.calculator.BadConfiguration: No configuration of aims
ERROR run_aims.py - ase.calculators.calculator.BadConfiguration: No configuration of aims
ERROR run_workflows_ReactAims.py - ase.calculators.calculator.BadConfiguration: No configuration of aims
ERROR run_workflows_ReactAims_parallel.py - ase.calculators.calculator.BadConfiguration: No configuration of aims

logsdail · 2024-08-05T06:47:36Z

[like] Andrew Logsdail reacted to your message:

…

________________________________ From: FirasAssa ***@***.***> Sent: Sunday, August 4, 2024 9:13:06 PM To: logsdail/carmm ***@***.***> Cc: Andrew Logsdail ***@***.***>; State change ***@***.***> Subject: Re: [logsdail/carmm] 30 update codebase to work with Python 3.8 (and thus ase v3.23) (PR #163) External email to Cardiff University - Take care when replying/opening attachments or links. Nid ebost mewnol o Brifysgol Caerdydd yw hwn - Cymerwch ofal wrth ateb/agor atodiadau neu ddolenni. During my project the following changes were made: The err_handler folder was removed and a utils folder was created in its place A python_env_check file was created with python_env_check() and is_env_python() inside Implemented python_env_check() in anylyse_forces example Implemented python_env_check() in analyse_calculator Restricted mace-torch to 0.3.4, because it was failing on v3.7 Updated CatLearn to the most recent version to fix import error in mlneb.py, by getting the version of CatLearn directly from Git Thinks still to do after the project: In python v3.8 ci-test there are 4 places where .BadConfiguration Error occurs. Presumably it is the same type of problem that can be fixed in one go Errors: ERROR analyse_counterpoise.py - ase.calculators.calculator.BadConfiguration: No configuration of aims ERROR run_aims.py - ase.calculators.calculator.BadConfiguration: No configuration of aims ERROR run_workflows_ReactAims.py - ase.calculators.calculator.BadConfiguration: No configuration of aims ERROR run_workflows_ReactAims_parallel.py - ase.calculators.calculator.BadConfiguration: No configuration of aims — Reply to this email directly, view it on GitHub<#163 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFMK4MYUDKHIE4IDSR2UDILZP2KOFAVCNFSM6AAAAABIWCQXZCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRXGY3TQOJWGE>. You are receiving this because you modified the open/close state.Message ID: ***@***.***>

logsdail added 2 commits June 3, 2024 10:08

Update main.yml

dcd297d

Update the yaml files so now we are testing ASE v3.23.0.

Update main.yml

747f4b0

v3.23.0 requires Python 3.8, so making this update.

logsdail linked an issue Jun 3, 2024 that may be closed by this pull request

Update codebase to work with ASE v3.23 #30

Closed

Update main.yml

4fa53b9

Backtracking, to test what behaviour we get with Python 3.7

logsdail changed the title ~~30 update codebase to work with ase v323~~ 30 update codebase to work with Python 3.8 (and thus ase v3.23) Jun 3, 2024

logsdail added 2 commits June 3, 2024 11:06

Update main.yml

1f0b9ce

Changed to version Python 3.8 - should we go higher?

Update linter.yml

419ec6f

Changed to version Python 3.8 - should we go higher?

logsdail added 12 commits June 5, 2024 18:34

Update linter.yml

4b56114

Remove some redundant comments.

Create linter-python38.yml

4b62b33

Added new file to just do Python 3.8 linter checks.

Update and rename linter.yml to linter-python37.yml

0008b42

Revert existing Linter to Python 3.7, and name appropriately (so can check and errors arising)

Update and rename main.yml to main-python37.yml

5ed3d4a

Updated CI tests to run for Python 3.7 explicit. Will duplicate for Python 3.8 and Python 3.9

Create main-python38.yml

2e58dff

Added Python 3.8 check

Delete .github/workflows/main-python38.yml

e370276

Realised I can just do this with a version matrix.

Delete .github/workflows/linter-python38.yml

122ad32

Realised I can do this with a version matrix.

Update and rename linter-python37.yml to linter.yml

da5f72e

Setup linter to run over multple Python versions. Probably overkill but good testbed

Update and rename main-python37.yml to main.yml

51cc159

Upgrade the CI tests (also has an action version update)

Merge branch 'main' into 30-update-codebase-to-work-with-ase-v323

986c94c

Trying to debug issues with counterpoise testing

587e236

Removed debug statements. The problem here is actually with the FHI-a…

29ce07a

…ims interface, and when the test fails it doesn't revert to the correct directory. Requires some careful thinking how to resolve @GaryLZW

logsdail mentioned this pull request Jun 19, 2024

Fix the changing directory problem. #169

Merged

logsdail and others added 6 commits June 19, 2024 18:39

Merge branch 'main' into 30-update-codebase-to-work-with-ase-v323

3d2e30e

Changed regression test for analyse_forces(), so it passes python 3.8

31108ec

Changed regression test for analyse_forces(), so it passes python 3.8

4328f1d

Created utils folder and moved logger into it

38c3a94

Created utils folder and moved logger into it

6168ef4

Created is_env_python() that checks if the python environment version…

f1445a7

… is true

logsdail and others added 6 commits July 31, 2024 16:29

Minor tweak to the analyse_force example, checking ASE version instea…

8615ddc

…d of Python

Counterpoise ase version 3.23 (#174)

2574d99

* Accommodate ase 3.23 * clean up some print statements * Change assertion line due to change of input file style. Write input files for 3.23 case --------- Co-authored-by: Andrew Logsdail <LogsdailA@cardiff.ac.uk>

Merge branch '30-update-codebase-to-work-with-ase-v323' of github.com…

470aee1

…:logsdail/carmm into 30-update-codebase-to-work-with-ase-v323

Tidying to pass the linter checks

edb4873

Changes in QA tests

de7abdd

Reduced complexity of testing to just Py3.7 and 3.8

5e8fdc5

logsdail requested review from GaryLZW, ikowalec, GabrielBram, AkashHiregange and FirasAssa August 1, 2024 07:23

ikowalec approved these changes Aug 1, 2024

View reviewed changes

logsdail added 3 commits August 1, 2024 11:12

Tidying some issues under review

032148f

Commenting in new functions

8bd3089

Removed remaining text in header

c704db1

logsdail added 2 commits August 1, 2024 12:58

Simplified the counterpoise correction CI test, as we shouldn't need …

a2f77ec

…to change calculator type here depending on ASE version

Fixing linter. I should learn to run code before committing

3d34bdb

GaryLZW reviewed Aug 2, 2024

View reviewed changes

carmm/run/aims_calculator.py Outdated Show resolved Hide resolved

logsdail added 3 commits August 2, 2024 13:30

Tidied calculator definitions. @GaryLZW is this clearer/better?

4689fc0

Tidied the run_aims example as noted we have better ways now to check…

c0c8eec

… the code version

Resolving minor issue with missing varaible

68e195b

GaryLZW approved these changes Aug 2, 2024

View reviewed changes

logsdail merged commit 08d248e into main Aug 2, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

30 update codebase to work with Python 3.8 (and thus ase v3.23) #163

30 update codebase to work with Python 3.8 (and thus ase v3.23) #163

logsdail commented Jun 3, 2024

codecov bot commented Jun 3, 2024 •

edited

Loading

logsdail commented Jun 3, 2024

logsdail commented Jun 5, 2024

logsdail commented Jun 5, 2024

logsdail commented Aug 1, 2024

ikowalec left a comment

ikowalec commented Aug 1, 2024

logsdail commented Aug 1, 2024

logsdail commented Aug 1, 2024

GaryLZW commented Aug 2, 2024

FirasAssa commented Aug 4, 2024

logsdail commented Aug 5, 2024 via email

30 update codebase to work with Python 3.8 (and thus ase v3.23) #163

30 update codebase to work with Python 3.8 (and thus ase v3.23) #163

Conversation

logsdail commented Jun 3, 2024

codecov bot commented Jun 3, 2024 • edited Loading

Codecov Report

logsdail commented Jun 3, 2024

logsdail commented Jun 5, 2024

logsdail commented Jun 5, 2024

logsdail commented Aug 1, 2024

ikowalec left a comment

Choose a reason for hiding this comment

ikowalec commented Aug 1, 2024

logsdail commented Aug 1, 2024

logsdail commented Aug 1, 2024

GaryLZW commented Aug 2, 2024

FirasAssa commented Aug 4, 2024

logsdail commented Aug 5, 2024 via email

codecov bot commented Jun 3, 2024 •

edited

Loading