TBStudio parser #154

JosePizarro3 · 2023-08-22T14:02:51Z

No description provided.

mohammadnakhaee · 2023-08-25T10:25:09Z

Hi @JosePizarro3 I added the test for the parser. Please take a look if we can merge it.

JosePizarro3

Very nice, just some more detail work and the parser will be ready.

I've only made an initial review, focusing on the parsing side. Once you have the new changes under control, I will turn to the specific parsing and metainfo definitions (everything that is inside parse_system, parse_method, parse_scc + the metadata in nomad).

Let me know if something is unclear.

electronicparsers/tbstudio/parser.py

electronicparsers/utils/utils.py

mohammadnakhaee · 2023-08-28T13:30:03Z

Hi @JosePizarro3 Please take a look on the last changes.

JosePizarro3

Hi,

Here you have a new round of comments. Mainly coding recommendations, a few checks, and some stuff that would make your life easier if you need to maintain this parser 🙂

Feel free to ask back. When you go resolving comments, feel free to drop a message like 'Done', or some more extended explanation, and resolve it yourself.

Thanks a lot for the hard work, the parser looks very nice! 👍🏻 And after these changes, I can review the nomad-gitlab side and we could merge soon.

electronicparsers/tbstudio/parser.py

JosePizarro3 · 2023-08-29T09:05:43Z

electronicparsers/utils/utils.py

+        # First Principles Calculation task
+        if self.archive.workflow2:
+            first_principles_task = TaskReference(task=first_principles_calculation_archive.workflow2)
+            first_principles_task.name = 'First-Principles Calculation'


The reason of naming a task with 'Calculation' might be confusing. Even thought we all know that calculation is what we do, in NOMAD we further define 'calculation' as containing the outputs of a given 'method' applied to a specific 'material'. Here we are talking about the global task (containing system, method, calculation), hence I think it makes more sense to simply call it 'First Principles'

electronicparsers/utils/utils.py

tests/test_tbstudioparser.py

JosePizarro3 · 2023-10-31T14:10:55Z

@mohammadnakhaee let me know when you need another quick round of review. I'll like to merge soon the new refactoring of 'Projection' to 'TB', https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/1285.

Maybe, we should decouple both changes: one merge for the renaming, another merge for adding SlaterKoster and TBStudio.

mohammadnakhaee · 2023-10-31T15:03:58Z

@JosePizarro3 yes. Could you please take a look. I solved the problems and lint errors related to tbstudio but it is still failed. Maybe you know what is going wrong. The failed tests are not related to the tbstudio.

JosePizarro3 · 2023-11-01T17:55:21Z

@JosePizarro3 yes. Could you please take a look. I solved the problems and lint errors related to tbstudio but it is still failed. Maybe you know what is going wrong. The failed tests are not related to the tbstudio.

Maybe rebasing would solve the issue?

Bear in mind that pytest will anyways fail. I'd say that if after rebasing pylint and mypy do not pass, I'll take a look 🙂

mohammadnakhaee · 2023-11-01T18:46:51Z

Yeah let me rebase it. Maybe it helps.

mohammadnakhaee · 2023-11-01T19:22:15Z

I rebased it. Now the problem is only these two

electronicparsers/utils/utils.py:30:0: E0611: No name 'TB' in module 'nomad.datamodel.metainfo.simulation.workflow' (no-name-in-module)
electronicparsers/utils/utils.py:30:0: E0611: No name 'TBMethod' in module 'nomad.datamodel.metainfo.simulation.workflow' (no-name-in-module)

which I guess it makes sense. Doesn't it? They are newly added in the gitlab branch which is not merged yet. Is it correct?

JosePizarro3 · 2023-11-02T06:33:41Z

which I guess it makes sense. Doesn't it? They are newly added in the gitlab branch which is not merged yet. Is it correct?

Yes, indeed 🙂

Perfect, I will take a look and give you some final review. Though it looked good at first sight. Do you want me also to review now the gitlab side?

mohammadnakhaee · 2023-11-02T08:38:26Z

Yes please see the gitlab again. I will close all the threads since there are extensive changes to the previous version. Would be nice if you comment on this version.

JosePizarro3

Hey, just recalling some suggestions and fixes from my previous review.

Furthermore, the data files are not present (they should be in tests/data/tbstudio).

Once these suggestions are implemented, this will be ready 🙂

JosePizarro3 · 2023-11-07T08:25:10Z

electronicparsers/tbstudio/parser.py

+        """
+        sec_run = self.archive.run[-1]
+
+        sec_run.program = Program(name='TBStudio', version=self.tbm['ReleaseVersion'])


Original suggestion:

Error handling:

sec_run.program = Program(name='TBStudio', version=self.tbm.get('ReleaseVersion', ''))

The problem is that, if self.tbm['ReleaseVersion'] is not stored (for whatever reason, like for example a version change), it will return an error. It is safer to do self.tbm.get('ReleaseVersion') or self.tbm.get('ReleaseVersion', '') if you want to be more verbose with the type of the get.

JosePizarro3 · 2023-11-07T08:26:00Z

electronicparsers/tbstudio/parser.py

+        """Populates run.calculation with the output of the calculation.
+        """
+        sec_run = self.archive.run[-1]
+        self.archive.run[-1].m_create(Calculation)


Original suggestion:

You can:

sec_scc = sec_run.m_create(Calculation)

And delete the line here where you do sec_scc = sec_run.calculation[-1] (see next comment).

Also reopening this suggestion for making parse_scc more readable.

JosePizarro3 · 2023-11-07T08:26:35Z

electronicparsers/tbstudio/parser.py

+            return
+
+        fermi_level_joules = self.tbm['variables']['ChemP'] * ureg.eV
+        sec_scc = sec_run.calculation[-1]


You can delete this when doing what I said in the comment before.

Here according to the suggestion of initializing sec_scc = sec_run.m_create(Calculation) at the beginning directly.

JosePizarro3 · 2023-11-07T08:26:42Z

electronicparsers/tbstudio/parser.py

+        if band_segments_points is None or len(tb_bands) < 1 or len(frac_k_points) < 1:
+            return
+
+        fermi_level_joules = self.tbm['variables']['ChemP'] * ureg.eV


Error handling (and no need to name 'joules'):

fermi_level = self.tbm['variables'].get('ChemP') sec_energy = sec_scc.m_create(Energy, Calculation.energy) sec_energy.fermi = fermi_level * ureg.eV if fermi_level else None

And this error handling too

electronicparsers/tbstudio/parser.py

JosePizarro3 · 2023-11-07T08:33:14Z

electronicparsers/tbstudio/parser.py

+        self.logger = logger if logger is not None else logging
+
+        sec_run = self.archive.m_create(Run)
+        sec_run.program = Program(name="TBStudio")


Furthermore, you were written the Program information in parse_method. Please, move the program definition here (deleting the corresponding lines in parse_method) doing:

sec_run.program = Program(name='TBStudio', version=self.tbm.get('ReleaseVersion', ''))

This, there are two calls for program which are overwritten each other.

JosePizarro3 · 2023-11-07T08:33:20Z

electronicparsers/tbstudio/parser.py

+        self.parse_method()
+        self.parse_scc()
+
+        self.parse_workflow()


No need to define a specific function here. Simply delete that method and do:

workflow = SinglePoint() self.archive.workflow2 = workflow

This

JosePizarro3 · 2023-11-07T08:33:38Z

electronicparsers/tbstudio/parser.py

+                pass
+            if first_principles_calculation_archive and self._child_archives:
+                tb_workflow_archive = self._child_archives.get('TB_workflow')
+                self.parse_tb_workflow(tb_archive, first_principles_calculation_archive, tb_workflow_archive)


No need to call tb_archive, but rather pass archive or self.archive

This

electronicparsers/utils/utils.py

JosePizarro3 · 2023-11-13T09:39:11Z

@mohammadnakhaee all good, come to my office (I have a final doubt) and we do the final merge together 🙂

…orkflow

get the labels from atomic numbers using ase library

use units converter instead of multiplying by numbers

JosePizarro3 added the feature / enhancement New feature or request label Aug 22, 2023

JosePizarro3 assigned mohammadnakhaee Aug 22, 2023

JosePizarro3 linked an issue Aug 22, 2023 that may be closed by this pull request

TBS parser #90

Closed

mohammadnakhaee force-pushed the tbs_parser branch from 8bdbd4c to 3ae46b2 Compare August 24, 2023 12:53

JosePizarro3 commented Aug 25, 2023

View reviewed changes

JosePizarro3 commented Aug 29, 2023

View reviewed changes

mohammadnakhaee force-pushed the tbs_parser branch from efa4b25 to d8568b5 Compare November 1, 2023 19:00

JosePizarro3 force-pushed the tbs_parser branch from d8568b5 to aa59df6 Compare November 2, 2023 14:37

JosePizarro3 commented Nov 7, 2023

View reviewed changes

JosePizarro3 force-pushed the tbs_parser branch from 52d8f2f to 712e206 Compare November 13, 2023 09:49

JosePizarro3 and others added 10 commits November 20, 2023 11:18

Initial commit TBS parser

1875eff

Changed name from TBS to TBStudio

8506d04

Added tight binding studio parser

a7e2b33

parse SK method

689733d

fix bugs

a2848e7

fix bugs

189079c

added program name

650564c

Initial commit TBS parser

3a7ac44

Changed name from TBS to TBStudio

70d2f65

Added tight binding studio parser

92ba6fd

mohammadnakhaee and others added 26 commits November 20, 2023 11:18

Added dft-to-tb workflow

90f9eba

compatiblity with the newest version

140834a

Fixed workflow import after rebasing

aa5e93d

fix bug

49bde2d

refactor TightBinding to TB in tbstudio and wannier90

6ad7d0f

parse sk integrals and orbitals

c09f42f

fix parser and the model

7977c27

show the tight-binding band-structure and the dft band-structure in w…

514c503

…orkflow

fix the band-structures in workflow and SinglePoint

bc8f4a9

added firstPrinciples-TB workflow

e103ff1

added fermi level

c0be7ba

fixed Tight-Binding BZ

3e96a78

Added the new workflow implementation

c96aa28

Added a test for parser

719110d

remove the middle parsed dictionary

93ca316

Harmonized the workflow name with the method name

d0b72c9

sec_atoms.labels is the correct atomic types

a1b8136

get the labels from atomic numbers using ase library

correct the name of the tasks to be consistent

ed2a852

test the labels instead of species

6309bac

use units converter instead of multiplying by numbers

add a default empty array for protection

da24d6d

fix lint

b9d9367

fix pylint

0b0d951

Added test tbstudio file

94c566e

Rafactor and Added error handling

4d06783

Remove unused import

f66e7fc

Fix Wannier90 method.tb

a6e2521

JosePizarro3 force-pushed the tbs_parser branch from 712e206 to a6e2521 Compare November 20, 2023 10:19

fix ruff

24b8c83

JosePizarro3 merged commit 4f54eef into develop Nov 20, 2023

JosePizarro3 deleted the tbs_parser branch November 20, 2023 10:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TBStudio parser #154

TBStudio parser #154

JosePizarro3 commented Aug 22, 2023

mohammadnakhaee commented Aug 25, 2023 •

edited

Loading

JosePizarro3 left a comment

mohammadnakhaee commented Aug 28, 2023

JosePizarro3 left a comment

JosePizarro3 Aug 29, 2023

JosePizarro3 commented Oct 31, 2023

mohammadnakhaee commented Oct 31, 2023

JosePizarro3 commented Nov 1, 2023

mohammadnakhaee commented Nov 1, 2023

mohammadnakhaee commented Nov 1, 2023

JosePizarro3 commented Nov 2, 2023

mohammadnakhaee commented Nov 2, 2023

JosePizarro3 left a comment

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 commented Nov 13, 2023

TBStudio parser #154

TBStudio parser #154

Conversation

JosePizarro3 commented Aug 22, 2023

mohammadnakhaee commented Aug 25, 2023 • edited Loading

JosePizarro3 left a comment

Choose a reason for hiding this comment

mohammadnakhaee commented Aug 28, 2023

JosePizarro3 left a comment

Choose a reason for hiding this comment

JosePizarro3 Aug 29, 2023

Choose a reason for hiding this comment

JosePizarro3 commented Oct 31, 2023

mohammadnakhaee commented Oct 31, 2023

JosePizarro3 commented Nov 1, 2023

mohammadnakhaee commented Nov 1, 2023

mohammadnakhaee commented Nov 1, 2023

JosePizarro3 commented Nov 2, 2023

mohammadnakhaee commented Nov 2, 2023

JosePizarro3 left a comment

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

JosePizarro3 commented Nov 13, 2023

mohammadnakhaee commented Aug 25, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading

JosePizarro3 Nov 7, 2023 •

edited

Loading