Elasticity tensor calculations using pymatgen.#211
Elasticity tensor calculations using pymatgen.#211AkashHiregange wants to merge 65 commits intotamm-cci:mainfrom
Conversation
… values now to avoid singularity.
…d the Atoms equilibrium structure that was being returned as it was redundant
…ucture. Also, added the flexibility to write files in different formats.
…n parameters. Now, it only need the output file extension.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
I know why you have put this in a new subdirectory, but I advocate that you split the files over the existing |
|
I'm not sure how important the additional todos are but my advice if you have a working example done is to merge, and then address the improvements in subsequent refinements (i.e., don't try and save everything for a mega-merge) |
|
BTW the commenting is excellent! |
… an argument for the function. If None, then os.getcwd() will be used.
…ite the calculation output if write_output=True
… that everything is consistent.
…also added the path argument to save the files.
…ed to ubuntu 22.04
…t and submission script included for CI tests
|
Alright, the CI tests finally work with all Python versions giving consistent values. However, there are a few caveats worth noting for future. This implementation leads to consistent values across Python versions. I honestly don't understand why this worked, as I would have expected the number of threads to be the same (i.e. defaults) for different runs. However, this does not completely eliminate variability. Even with identical environment variables, different GitHub runners (e.g. Ubuntu 22.04 vs Ubuntu 24.04) still produce slightly different numerical results. So, I had to change the Ubuntu version to 22.04 (as there is no Python3.7 on 22.04) to make sure the tests pass consistently (see commit ID: b9f55e1) I will raise an issue for the same in Pymatgen source repository. However, this raises an important question from my side. Do we still need Python3.7? I checked that the minimum version of Python on the supercomputers that we currently use is 3.9. This means we don't have to rely on Ubuntu 22.04 in our CI tests. |
|
Although my previous comment was based on my tests, my recent commit (0809de4) shows that the real issue was the version of Ubuntu. In this commit, I removed the environment variables and the CI tests still pass. This shows that the real issue was Ubuntu 22.04 v/s 24.04. I think that the default environment variables for OPENBLAS threading is different in both the versions, leading to the issues we observed. This motivates us again to ask the question 'do we need Python3.7?' |
|
The CI tests pass for newer versions of Python without explicitly setting the environments. Good to merge from my side if there are no further comments :) |
|
I'm happy with all this - you've done a significant amount of work and also been a good detective on resolving issues! As this is a big commit and I've already checked, I would suggest we get a once over from someone else before merging away - I see @OscarvanVuren @robinsonmt1 and @GaryLZW are tagged? |
|
I also don't understand if we have removed 3.7/3.8 from the CI tests, why this is still coming up as part of the required tests for merge? Is there something missing in the yaml updates? |
There was a problem hiding this comment.
Is this file really necessary? Given it is highly specific to Isambard3 and just calls python3 on the calculation script
There was a problem hiding this comment.
This file was added to make sure that the following lines in get_deformed_structures_for_elasticity_tensor.py gets covered during the CI and code-coverage tests. I have put a comment in the input.py saying that the files should be replaced by the user for their use case.
if copy_input_and_submission:
shutil.copy(home + '/input.py', path_final + '/input.py')
shutil.copy(home + '/submission.script', path_final + '/submission.script')
There was a problem hiding this comment.
I see. Then probably the names of these scripts should not be hardcoded
There was a problem hiding this comment.
Actually, I understand now. There needs to be something for shutil to copy to test the functionality, it does not matter what this actually is. Whilst input.py and submission.script should not be hardcoded, the files are necessary to include. Could they be replaced with dummy files?
There was a problem hiding this comment.
The input.py is actually a dummy file. It has no code, except a print statement that indicates that it is a dummy file. I will change the submission.script to a dummy one
From what I understand, this seems to be a problem in github caching. I have looked over the main and linter file, but could not find any residuals of 3.7/3.8 left behind. |
|
[like] Andrew Logsdail reacted to your message:
…________________________________
From: AkashHiregange ***@***.***>
Sent: Thursday, March 19, 2026 12:15:30 PM
To: tamm-cci/carmm ***@***.***>
Cc: Andrew Logsdail ***@***.***>; Comment ***@***.***>
Subject: Re: [tamm-cci/carmm] Elasticity tensor calculations using pymatgen. (PR #211)
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.
[https://avatars.githubusercontent.com/u/121492505?s=20&v=4]AkashHiregange left a comment (tamm-cci/carmm#211)<#211 (comment)>
I also don't understand if we have removed 3.7/3.8 from the CI tests, why this is still coming up as part of the required tests for merge? Is there something missing in the yaml updates?
From what I understand, this seems to be a problem in github caching. I have looked over the main and linter file, but could not find any residuals of 3.7/3.8 left behind.
—
Reply to this email directly, view it on GitHub<#211 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFMK4MYIRVIAH27LYSHHZPT4RPQGFAVCNFSM6AAAAACOFWUD6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAOBZG4YTIMJTGI>.
You are receiving this because you commented.Message ID: ***@***.***>
|
The functionality makes use of pymatgen to create deformed geometries in separate folders from a set of normal and shearing strains. Having performed first-principles/force-field calculations on these displaced structure to compute the stress tensors,, the functionality also allows a seamless post-processing involving compilation of stress tenors from the first-principles/force-field output, and computing the fourth-rank elasticity tensor. Currently, the work-flow relies on the user performing first-principles/force-field calculations on individual deformed structures.
TODOs:
• create an example (MACE might be probably quicker and we can use a jupyter notebook to run the example)
• Need modify the function so that correct charges and moments are fed to the deformed structures.
• Allow flexibility to compute elasticity tensor in different units.
• Flexibility to read stress tensors from the QM calculators like VASP and make sure that the units are consistent