use `orjson` to store metainfo package #613

lukaspie · 2025-04-04T15:17:11Z

No description provided.

lukaspie · 2025-04-07T14:24:00Z

When loading the pynxtools metainfo package from the stored file src/pynxtools/nomad/nxs_metainfo_package.json (which is autogenerated from running the schema without re-loading), it looks like the metainfo is correctly instantiated.

However, in parsing, there is an error along the lines of There is no subsection to hold a pynxtools.nomad.schema.<section-name>:Section in pynxtools.nomad.schema.<section-parent-name>:Section. For the specific debugging I am using (see launch.json, I get the error There is no subsection to hold a pynxtools.nomad.schema.Arpes__ENTRY:Section in pynxtools.nomad.schema.Arpes:Section. This is the first node in the NeXus files that's parsed. Interestingly, additional attributes that are in the NeXus file (in the NeXus base class NXroot) are properly parsed, so something is working.

For the error above, it basically comes from m_create function call in src/pynxtools/nomad/parser.py, specifically from this line. Interestingly, in debug mode, self.m_def.all_sub_sections_by_section does contain the value of section_def (which is pynxtools.nomad.schema.Arpes__ENTRY:Section) as a key, see the screenshot.

So basically, my main questions are:

Why does the get call not resolve even though the section is in the all_sub_sections_by_section?
What's different about instantiating the schema from scratch vs. loading the package and initializing the metainfo afterwards? If I do the former, everything looks exactly the same in debug mode, but the get call keeps properly resolved.

@lauri-codes @blueraft @DanielYang59 any suggestions why this may be the case? cc @sanbrock

lukaspie · 2025-04-08T07:55:04Z

Problem is that these two objects are not exactly the same. In debug mode,

id(list(self.m_def.all_sub_sections_by_section.keys())[-1])
>> 140248739911328
id(section_def)
>> 140248775424816

On the other hand, if we generate the schema from scratch, they are indeed the same object:

id(list(self.m_def.all_sub_sections_by_section.keys())[-1])
>> 140336944267776
id(section_def)
>> 140336944267776

So it seems that somewhere in the serialization and reloading of the schema the memory address changes, but I don't understand this well enough.

We reproduced the same error when using a small test json file that only contains the section definitions needed for reproducing the error, see attached. In order to check this schema, you need to replace the name in the load_nexus_schema function in the schema.py file.

test.json

blueraft · 2025-04-08T12:26:18Z

I can also reproduce this! cc: @TLCFEM since this is something related to the metainfo.

TLCFEM · 2025-04-08T19:49:23Z

Try set cached=False in this line: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/blame/develop/nomad/metainfo/metainfo.py?ref_type=heads#L4394
Report back the result to proceed further.

lukaspie · 2025-04-09T08:20:49Z

Try set cached=False in this line: gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/blame/develop/nomad/metainfo/metainfo.py?ref_type=heads#L4394 Report back the result to proceed further.

I still get the same issue, the ids do not match. I also tried cached=False for all_sub_sections since that is called in all_sub_sections_by_section, but the result was the same.

TLCFEM · 2025-04-09T11:09:01Z

@lukaspie Please apply and check if everything works.
https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/2430

TLCFEM · 2025-04-09T18:06:09Z

Long story short, m_from_dict utilise the SectionReference to resolve the references section, this yields a MProxy that mimics the behaviour of a Section.
It contains everything the target Section contains but they are different objects in memory.

src/pynxtools/nomad/nxs_metainfo_package.json

src/pynxtools/nomad/schema.py

blueraft · 2025-04-10T09:34:43Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

lukaspie · 2025-04-10T10:32:17Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

I think the original idea was that pynxtools can be used standalone (for data conversion), so it was meant to be a lightweight install outside of NOMAD. I think after pluginization, we should have probably added nomad as explicit dependency though.

In any case, we can probably only publish after the next nomad release anyway, right?

rettigl · 2025-04-10T10:41:25Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

I think the original idea was that pynxtools can be used standalone (for data conversion), so it was meant to be a lightweight install outside of NOMAD. I think after pluginization, we should have probably added nomad as explicit dependency though.

In any case, we can probably only publish after the next nomad release anyway, right?

I would argue we still should keep this independence. There should definitely be a standalone installation without nomad possible. We could create an optional dependency group, e.g. [nomad] that defines the required nomad version, though.

lukaspie · 2025-04-10T11:13:00Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

I think the original idea was that pynxtools can be used standalone (for data conversion), so it was meant to be a lightweight install outside of NOMAD. I think after pluginization, we should have probably added nomad as explicit dependency though.
In any case, we can probably only publish after the next nomad release anyway, right?

I would argue we still should keep this independence. There should definitely be a standalone installation without nomad possible. We could create an optional dependency group, e.g. [nomad] that defines the required nomad version, though.

So, we could have a [nomad] extra in pynxtools pyproject.toml and then in nomad-distro, we would use pynxtools[nomad]==<some-version> to ensure compatibility?

If I remember correctly, there were problems when using optional dependencies (like we had before with [convert]) and then running using uv pip compile like here, but maybe this is different.

blueraft · 2025-04-10T11:15:47Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

I think the original idea was that pynxtools can be used standalone (for data conversion), so it was meant to be a lightweight install outside of NOMAD. I think after pluginization, we should have probably added nomad as explicit dependency though.
In any case, we can probably only publish after the next nomad release anyway, right?

I would argue we still should keep this independence. There should definitely be a standalone installation without nomad possible. We could create an optional dependency group, e.g. [nomad] that defines the required nomad version, though.

So, we could have a [nomad] extra in pynxtools pyproject.toml and then in nomad-distro, we would use pynxtools[nomad]==<some-version> to ensure compatibility?

If I remember correctly, there were problems when using optional dependencies (like we had before with [convert]) and then running using uv pip compile like here, but maybe this is different.

In pynxtools[nomad], nomad-lab >= 1.3.16 should work though, having a fixed version there would instead complicate things.

rettigl · 2025-04-10T11:21:50Z

In pynxtools[nomad], nomad-lab >= 1.3.16 should work though, having a fixed version there would instead complicate things.

That's more or less what I mean, yes. Not sure about the uv pip compile, though. I guess we need to test it.

lukaspie · 2025-04-10T11:29:31Z

With this PR, the next pynxtools release would be incompatible with earlier versions of nomad right? Since pynxtools doesn't declare nomad as a direct dependency (not sure why?), this might result in some broken installations if they're not using the latest nomad version.

I think the original idea was that pynxtools can be used standalone (for data conversion), so it was meant to be a lightweight install outside of NOMAD. I think after pluginization, we should have probably added nomad as explicit dependency though.
In any case, we can probably only publish after the next nomad release anyway, right?

I would argue we still should keep this independence. There should definitely be a standalone installation without nomad possible. We could create an optional dependency group, e.g. [nomad] that defines the required nomad version, though.

So, we could have a [nomad] extra in pynxtools pyproject.toml and then in nomad-distro, we would use pynxtools[nomad]==<some-version> to ensure compatibility?
If I remember correctly, there were problems when using optional dependencies (like we had before with [convert]) and then running using uv pip compile like here, but maybe this is different.

In pynxtools[nomad], nomad-lab >= 1.3.16 should work though, having a fixed version there would instead complicate things.

Yeah sorry, this is what I meant, just a lower bound.

lukaspie · 2025-04-11T09:23:08Z

@rettigl I have implemented it now that the json file is created before the uv build step is run, so it gets shipped with the package to PyPI and is available after install. For editable installs, on the first run, we generate the package once and then afterwards, we can just reload it. The filename also contains the NeXus version, so if you change the definition, the schema package is automatically recreated. Are you happy with that? We also have nomad in the optional-dependencies now.

rettigl · 2025-04-11T11:09:32Z

@rettigl I have implemented it now that the json file is created before the uv build step is run, so it gets shipped with the package to PyPI and is available after install. For editable installs, on the first run, we generate the package once and then afterwards, we can just reload it. The filename also contains the NeXus version, so if you change the definition, the schema package is automatically recreated. Are you happy with that? We also have nomad in the optional-dependencies now.

This is totally fine. I only think it would be good to avoid doing every time you start up nomad, which already takes quite long for building the nexus metainfo.

sanbrock · 2025-04-13T19:16:46Z

.github/workflows/publish.yml

+            echo "ERROR: nxs_metainfo_package_*.json not found in installed package."
+            exit 1
+          else
+            echo "Found $FILE_COUNT JSON file(s) in package:"


shouldn't we test if the found package(s) correspond(s) to the actual definitions submodule?

sanbrock · 2025-04-13T19:25:49Z

scripts/update_definitions.sh

+# Updates the definitions and generates the metainfo package.
+# Only works if you have pynxtools installed in editable mode.
+
+update_nexus_version() {


Cannot we either put the common functions to a common file being sourced, or make a generic script which behaves differently based on a parameter (update vs. reset)?

changed to a single script with a parameter

sanbrock · 2025-04-13T19:31:48Z

src/pynxtools/nomad/schema.py

-    figure_host = archive.data
-    # apend the figure to the figures list
-    figure_host.figures = [PlotlyFigure(figure=fig.to_plotly_json())]
+    for section in sections:


would we gain additional time, if these attributes are also saved/loaded?

Thy are saved/loaded with this approach.

lukaspie changed the title ~~use orjson to store metainfo package~~ use orjson to store metainfo package Apr 4, 2025

lukaspie force-pushed the metainfo-pkg branch from dfa9c5f to db5d851 Compare April 7, 2025 12:44

lukaspie force-pushed the metainfo-pkg branch from bc402cd to fe3b16a Compare April 9, 2025 11:36

lukaspie marked this pull request as ready for review April 9, 2025 13:37

TLCFEM reviewed Apr 10, 2025

View reviewed changes

src/pynxtools/nomad/nxs_metainfo_package.json Outdated Show resolved Hide resolved

TLCFEM reviewed Apr 10, 2025

View reviewed changes

src/pynxtools/nomad/schema.py Outdated Show resolved Hide resolved

TLCFEM reviewed Apr 10, 2025

View reviewed changes

src/pynxtools/nomad/schema.py Show resolved Hide resolved

lukaspie force-pushed the metainfo-pkg branch 2 times, most recently from 0ae4739 to ca15b4f Compare April 10, 2025 13:05

lukaspie force-pushed the metainfo-pkg branch 2 times, most recently from a208554 to eae2e5f Compare April 11, 2025 09:47

lukaspie temporarily deployed to pypi April 11, 2025 10:47 — with GitHub Actions Inactive

lukaspie had a problem deploying to pypi April 11, 2025 11:00 — with GitHub Actions Failure

lukaspie had a problem deploying to pypi April 11, 2025 11:02 — with GitHub Actions Failure

lukaspie had a problem deploying to pypi April 11, 2025 11:13 — with GitHub Actions Failure

lukaspie temporarily deployed to pypi April 11, 2025 11:25 — with GitHub Actions Inactive

sanbrock reviewed Apr 13, 2025

View reviewed changes

lukaspie linked an issue Apr 23, 2025 that may be closed by this pull request

Lazy load of pynxtools schema in NOMAD #574

Open

lukaspie force-pushed the metainfo-pkg branch from 167d4ef to 971c7b8 Compare May 8, 2025 12:31

lukaspie force-pushed the metainfo-pkg branch 3 times, most recently from 118a5ec to 6286ec1 Compare June 10, 2025 11:09

lukaspie added 15 commits June 30, 2025 13:49

use orjson to store metainfo package

178c471

install packages in metainfo pkg workflow

f195eea

specify paths for package check

dc103cf

do not track package json file, install in publish

ed2ddc6

updating scripts for development

430c3fe

post install check that json file is in package

b7496c9

use nomad-dev release

9949e2d

use gitlab index-url for dev nomad

9190a5f

drop spm in convert extra for now

75016a1

remove support for python <3.10

d2456fa

use nomad extra when generating package in CI/CD

6e45053

install uv in test-install step

0c47817

only upload if test-install works

3e57024

attach normalizers after init

277b3c0

use a single definitions.sh script

c66a9e3

lukaspie force-pushed the metainfo-pkg branch 2 times, most recently from 9bb456e to eb4aea2 Compare June 30, 2025 12:11

ruff fixes for python>3.10

4d4c570

lukaspie force-pushed the metainfo-pkg branch from eb4aea2 to 4d4c570 Compare June 30, 2025 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

use `orjson` to store metainfo package #613

use `orjson` to store metainfo package #613

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

use orjson to store metainfo package #613

Are you sure you want to change the base?

use orjson to store metainfo package #613

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

use `orjson` to store metainfo package #613

use `orjson` to store metainfo package #613