Fix setup and add new schema #1

JosePizarro3 · 2024-07-24T10:08:47Z

@jkshenton Sathya (sorry, I cannot tag him as he is not part of this repo),

I've been working on some changes in this repo. Maybe you can drop me a review so I can merge and add this new plugin into NOMAD? You can also add it then as part of the NOMAD OASIS and use it in your CCP-NC database.

I still need to:

Add testing for the parser under tests/test_parser.py
Fix workflow recognition with CASTEP output files

Changes 1 - new structure

I've adopted the new structure for plugins from NOMAD. This means a lot of changes, but the most important are the ones in pyproject.toml and the new folder structure under src. Some extra stuff:

You can release this project in PyPI and maintain it with versioning in the way you want.
I might need to do minor changes from time to time to reflect changes in nomad.
I added instructions in the README.md, so you can test if these work or not for you.
I added a MkDocs page with a NOMAD style, that you can of course delete or restructure at will.

Changes 2 - New schema `nomad-simulations`

I've changed the parsing to the new schema, nomad-simulations. Some points here:

You are welcome to adopt or not this schema, as we already talked. If you need more quantities or a different way of parsing, I suggest you work from something up. The example here would be how I used Outputs from the nomad-simulations schema to extend it with NMR specific properties, and how I used PhysicalProperty from this package to define such NMR properties.
Now, I think the properties are much nicer parsed, as they contained references to the specific atomic states, more similar to what you have in the magres file.
In FAIRmat we are developing right now a taxonomy for materials properties, so anyways these properties might end up defined in the nomad or nomad-simulations repo, as some other users might be using these anyways.
As you can see, these properties have some functionalities to extract information (like the isotropic part of the MagneticShielding tensor), and these could be extended. I left it for you to decide whether you want to do this here or in some other repo you have.
Ideally, and to keep a nice collaboration, these properties could be perfectly defined in the nomad-simulations repo. I will be happy if you want to contribute there and update these definitions in there. Other users might need them and it connects nicely with the taxonomy effort I comment on above.

jkshenton · 2024-07-24T11:07:28Z

Thanks a lot @JosePizarro3 - really appreciate your continued help with this! The restructuring looks fine to me. The new schema (atoms state thing) will be much easier for us to work with I suspect and in general makes a lot of sense.

In terms of contributing to nomad-simulations, we are absolutely keen. The magres format spec document was already some effort to defining a standard schema for computed magres data for the community. I would be very happy to see it adapted for nomad generally. I count >13 periodic electronic structure codes that can calculate magnetic shielding tensors (one of the key properties) so having a general schema that other parsers can work with makes sense to me.

I've manually added @Sathya-S3 now and I think he's in a better position to review the specific code changes having tested out the initial version.

JosePizarro3 · 2024-07-24T15:28:58Z

Great, thanks @jkshenton !

In terms of contributing to nomad-simulations, we are absolutely keen. The magres format spec document was already some effort to defining a standard schema for computed magres data for the community. I would be very happy to see it adapted for nomad generally. I count >13 periodic electronic structure codes that can calculate magnetic shielding tensors (one of the key properties) so having a general schema that other parsers can work with makes sense to me.

Indeed, the magres format is a very easy case where, whatever you decided for the properties should be incorporated directly, while for the symmetry open questions, we should work a bit more on it.

Added nomad-lab[infrastructure] extra dependencies Added workflow for NMR CASTEP or QE + magres entry

Sathya-S3

Thank you for your work on presenting the magres parser in such a polished state, Jose. My main comments are related to understanding the workflow links between CASTEP and magres files, which I have highlighted with the relevant code. I'm happy to approve the pull request after we have had a chance to chat with you about the workflow linking and how we can help with further magres parser development (Quantum Espresso workflows etc.).

I have integrated the magres parser as guided by the README instructions with a NOMAD Oasis instance and have verified that the file parsing works flawlessly. The test was performed with an ethanol CASTEP-magres workflow files archive.

pyproject.toml

Sathya-S3 · 2024-08-29T14:09:37Z

pyproject.toml

-"Documentation" = "https://nomad-coe.github.io/nomad-parser-magres/"
+"Homepage" = "https://github.com/CCP-NC/nomad-parser-magres"
+"Bug Tracker" = "https://github.com/CCP-NC/nomad-parser-magres/issues"
+"Documentation" = "https://CCP-NC.github.io/nomad-parser-magres/"


The documentation page "https://CCP-NC.github.io/nomad-parser-magres/" does not exist at the time of review. I assume this will be auto-rectified when the changes are merged into "master branch". Will run a quick verification after merge.

we solved this last time we talk, the docu page has to be deployed in the settings of the repo. Check this: https://www.mkdocs.org/user-guide/deploying-your-docs/#github-pages

@jkshenton for your attention

src/nomad_parser_magres/parsers/parser.py

Sathya-S3 · 2024-08-29T22:30:27Z

src/nomad_parser_magres/parsers/parser.py

+        if outputs is not None:
+            simulation.outputs.append(outputs)
+
+        # Try to resolve the `entry_id` and `mainfile` of other entries in the upload to connect the magres entry with the CASTEP or QuantumESPRESSO entry


In the metadata section of file upload, there is a files section that lists all files included in the upload. Is this the only intended result? There is mention of linking the NMR workflow directly in the magres entry, could you please clarify how it is done? As per my previous comments, we see two separate file entries and workflows for the .castep and .magres files.

These lines are more to connect the castep with the magres entries

src/nomad_parser_magres/schema_packages/workflow.py

README.md

JosePizarro3 · 2024-09-10T06:22:23Z

@Sathya-S3 I will do a final clean up next week and push again. I think I can integrate your comments.

JosePizarro3 · 2024-10-30T10:36:47Z

I totally forgot about this PR. @Sathya-S3 reminded me and asked me a couple of things, so the problem is that the workflow might have some issues. I can follow FAIRmat-NFDI/nomad-parser-wannier90#3 to fix it and this will be ready.

Added new CCPNCSimulation class Fix dependencies Added get_files utility function Fix README

JosePizarro3 · 2024-10-31T13:23:19Z

@Sathya-S3 @jkshenton

Ok, I finished with some changes. Note the changes in src/schema_packages/ccpnc_metada.py, src/schema_packages/package.py and the last lines in src/parsers/parser.py. What I did:

In ccpnc_metadata.py: fix some types (instead of tuples use JSON) and shapes (if the shape is empty, no need to define). Add 2 comments.
In package.py: add CCPNCSimulation at the end.
In parser.py: change import to use CCPNCSimulation instead of nomad_simulations.Simulation. Also, I added some lines at the end of the file to clarify how the parsing of the extra CCP-NC metadata could look like; if you decide to go with another option, that's fine for me as well :-)

If you both agree, we can merge this.

jkshenton · 2024-10-31T13:52:37Z

Thanks so much again for your help with this, @JosePizarro3 ! I am happy to merge now and make any further changes in subsequent PRs.

JosePizarro3 added 8 commits July 22, 2024 16:05

Initial structuring

b069ab1

Added nmr output properties into the package definitions

7e18ae9

Deleted legacy runschema

c82277a

Added parsing into the new nomad-simulations schema

251a127

Added docstrings

c33b475

Adopting child_archives matching interface

1de95c0

Final touches on package.py

151cefb

Fixed github ci/cd

434e03d

JosePizarro3 requested review from jkshenton and Sathya-S3 July 24, 2024 11:41

JosePizarro3 self-assigned this Jul 24, 2024

JosePizarro3 added 5 commits July 25, 2024 09:27

Added tests

b554aa9

Fix mypy

b3dd0d3

Added automatic connection with CASTEP entries

0d9ee41

Added nomad-lab[infrastructure] extra dependencies Added workflow for NMR CASTEP or QE + magres entry

Added docstring to init_parser

5e1dbd3

Fix import circular problem

576057e

Sathya-S3 reviewed Aug 29, 2024

View reviewed changes

Sathya-S3 reviewed Aug 30, 2024

View reviewed changes

README.md Show resolved Hide resolved

Sathya-S3 and others added 3 commits October 30, 2024 10:45

Adding custom metadata

c1896ce

Fixed ccpnc_metadata schema

da1bbcf

Added new CCPNCSimulation class Fix dependencies Added get_files utility function Fix README

Fix ruff

7548fb8

Sathya-S3 merged commit 831f3f0 into develop Oct 31, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix setup and add new schema #1

Fix setup and add new schema #1

JosePizarro3 commented Jul 24, 2024 •

edited

Loading

jkshenton commented Jul 24, 2024

JosePizarro3 commented Jul 24, 2024

Sathya-S3 left a comment •

edited

Loading

Sathya-S3 Aug 29, 2024

JosePizarro3 Sep 9, 2024

Sathya-S3 Sep 16, 2024

Sathya-S3 Aug 29, 2024

JosePizarro3 Sep 9, 2024

JosePizarro3 commented Sep 10, 2024

JosePizarro3 commented Oct 30, 2024

JosePizarro3 commented Oct 31, 2024

jkshenton commented Oct 31, 2024

Fix setup and add new schema #1

Fix setup and add new schema #1

Conversation

JosePizarro3 commented Jul 24, 2024 • edited Loading

Changes 1 - new structure

Changes 2 - New schema nomad-simulations

jkshenton commented Jul 24, 2024

JosePizarro3 commented Jul 24, 2024

Sathya-S3 left a comment • edited Loading

Choose a reason for hiding this comment

Sathya-S3 Aug 29, 2024

Choose a reason for hiding this comment

JosePizarro3 Sep 9, 2024

Choose a reason for hiding this comment

Sathya-S3 Sep 16, 2024

Choose a reason for hiding this comment

Sathya-S3 Aug 29, 2024

Choose a reason for hiding this comment

JosePizarro3 Sep 9, 2024

Choose a reason for hiding this comment

JosePizarro3 commented Sep 10, 2024

JosePizarro3 commented Oct 30, 2024

JosePizarro3 commented Oct 31, 2024

jkshenton commented Oct 31, 2024

JosePizarro3 commented Jul 24, 2024 •

edited

Loading

Changes 2 - New schema `nomad-simulations`

Sathya-S3 left a comment •

edited

Loading