MPES reader cha 10000 nges from mpes refactoring #203

domna · 2024-01-08T13:10:03Z

This

Updates the reader to work with the new nexus definitions from NXmpes refactoring nexus_definitions#52
Updates the nexus definitions submodule
Temporarily disables the XPSReader test (it should then be re-activated from XPS reader: update to new NXmpes after refactor #207 @lukaspie)
Bugfixes in pynxtools to support new syntax first used in the mpes refactoring
Adds the feature to build a nested config file for the mpes reader

Fixes #210, Fixes #212

domna · 2024-01-17T17:14:53Z

Hey @rettigl, if you like you can already review the config_file.json. This is finished. I still need to update the regression tests and there are still some bugs in pynxtools (#210, #212) , which we need to fix before all tests pass.

I introduced a new feature that the config file can be written in a nested fashion, because I thought this is much easier to read and maintain (and we could easier show an "instance" of data at the workshop). I also added support for *{a,b,c} syntax, which basically replaces all * by the list in brackets, here a, b and c. You can see an example for the lenses, where it just expands for each of the lens labels. I checked the function such that it reproduces the old config, so we should be good. Links are now written as @link:<target> instead of a dict, as this is also more consistent with the file referencing. Except for the new link syntax everything should be compatible to the old flat json notation.

…fferent name

…concept name included

coveralls · 2024-01-18T17:09:58Z

Pull Request Test Coverage Report for Build 7585289570

58 of 60 (96.67%) changed or added relevant lines in 6 files are covered.
428 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-3.7%) to 40.812%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pynxtools/dataconverter/readers/utils.py	33	35	94.29%

Files with Coverage Reduction	New Missed Lines	%
pynxtools/dataconverter/readers/mpes/reader.py	3	83.33%
pynxtools/dataconverter/readers/xps/reader_utils.py	10	25.32%
pynxtools/dataconverter/readers/xps/file_parser.py	18	36.21%
pynxtools/dataconverter/readers/xps/reader.py	155	15.71%
pynxtools/dataconverter/readers/xps/xml/xml_specs.py	242	7.83%

Totals
Change from base Build 7567202073:	-3.7%
Covered Lines:	4471
Relevant Lines:	10955

💛 - Coveralls

rettigl

Generally, this looks quite good to me. However, there are a lot of "NOT IN SCHEMA" entries in the log files, some of them new, some of them also old. I think this should be carefully checked if this is supposed to be like this...

If I run the conversion in the tutorials E1 notebook, with definitions on "fairmat" branch, I get the following errors:

The data entry corresponding to /ENTRY[entry]/INSTRUMENT[instrument]/ELECTRONANALYSER[electronanalyser]/ENERGYDISPERSION[energydispersion]/energy_scan_mode is required and hasn't been supplied by the reader.
[info]: Path /ENTRY[entry]/PROCESS[process]/energy_calibration/coefficients not found. Skipping the entry.
The required group, /ENTRY[entry]/SAMPLE[sample]/bias/potentiostat, hasn't been supplied while its optional parent, /ENTRY[entry]/SAMPLE[sample]/bias, is supplied.

Still, the file is being created.
If I run with "mpes-fixes" branch, I get:

The data entry corresponding to /ENTRY[entry]/INSTRUMENT[instrument]/ELECTRONANALYSER[electronanalyser]/ENERGYDISPERSION[energydispersion]/energy_scan_mode is required and hasn't been supplied by the reader.
[info]: Path /ENTRY[entry]/PROCESS[process]/energy_calibration/coefficients not found. Skipping the entry.

tests/data/dataconverter/readers/mpes/Ref_nexus_mpes.log

tests/data/nexus/Ref_nexus_test.log

tests/nexus/test_nexus.py

rettigl · 2024-01-18T20:53:47Z

Conversion in E2 fails with:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[36], [line 1](vscode-notebook-cell:?execution_count=36&line=1)
----> [1](vscode-notebook-cell:?execution_count=36&line=1) convert(input_file=["config_file.json", "WSe2_eln.yaml"],
      [2](vscode-notebook-cell:?execution_count=36&line=2)         objects=res_xarray,
      [3](vscode-notebook-cell:?execution_count=36&line=3)         reader='mpes',
      [4](vscode-notebook-cell:?execution_count=36&line=4)         nxdl='NXmpes',
      [5](vscode-notebook-cell:?execution_count=36&line=5)         output='WSe2.mpes.nxs')

File /mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:269, in convert(input_file, reader, nxdl, output, generate_template, fair, undocumented, **kwargs)
    [264](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:264)         logger.info(
    [265](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:265)             f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation."
    [266](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:266)         )
    [268](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:268) helpers.add_default_root_attributes(data=data, filename=os.path.basename(output))
--> [269](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:269) Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write()
    [271](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/convert.py:271) logger.info(f"The output file generated: {output}.")

File /mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:339, in Writer.write(self)
    [337](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:337) """Writes the NeXus file with previously validated data from the reader with NXDL attrs."""
    [338](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:338) try:
--> [339](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:339)     self._put_data_into_hdf5()
    [340](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:340) finally:
    [341](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:341)     self.output_nexus.close()

File /mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:302, in Writer._put_data_into_hdf5(self)
    [296](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:296)         raise IOError(
    [297](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:297)             f"Unknown error occured writing the path: {path} "
    [298](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:298)             f"with the following message: {str(exc)}"
    [299](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:299)         ) from exc
    [301](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:301) for links in hdf5_links_for_later:
--> [302](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:302)     dataset = handle_dicts_entries(*links)
    [303](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:303)     if dataset is None:
    [304](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:304)         # If target of a link is invalid to be linked
    [305](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:305)         del self.data[links[-1]]

File /mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:146, in handle_dicts_entries(data, grp, entry_name, output_path, path)
    [144](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:144) elif "link" in data.keys():
    [145](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:145)     if ":/" not in data["link"]:
--> [146](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:146)         grp[entry_name] = h5py.SoftLink(path)  # internal link
    [147](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:147)     else:
    [148](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/pynxtools/pynxtools/dataconverter/writer.py:148)         grp[entry_name] = h5py.ExternalLink(file, path)  # external link

File h5py/_objects.pyx:54, in h5py._objects.with_phil.wrapper()

File h5py/_objects.pyx:55, in h5py._objects.with_phil.wrapper()

File /mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:929, in Dataset.__setitem__(self, args, val)
    [925](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:925) else:
    [926](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:926)     # If the input data is already an array, let HDF5 do the conversion.
    [927](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:927)     # If it's a list or similar, don't make numpy guess a dtype for it.
    [928](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:928)     dt = None if isinstance(val, numpy.ndarray) else self.dtype.base
--> [929](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:929)     val = numpy.asarray(val, order='C', dtype=dt)
    [931](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:931) # Check for array dtype compatibility and convert
    [932](https://vscode-remote+ssh-002dremote-002bpcr840-002egnz-002empg-002ede.vscode-resource.vscode-cdn.net/mnt/pcshare/users/Laurenz/AreaB/pynxtools/.pyenv/lib/python3.8/site-packages/h5py/_hl/dataset.py:932) if self.dtype.subdtype is not None:

TypeError: float() argument must be a string or a number, not 'SoftLink'

rettigl · 2024-01-18T21:07:07Z

This happens at

because of

I think we should add a test that the writer does not try to create a link if the link target does not exist...

pynxtools/dataconverter/helpers.py

domna · 2024-01-19T10:11:10Z

Generally, this looks quite good to me. However, there are a lot of "NOT IN SCHEMA" entries in the log files, some of them new, some of them also old. I think this should be carefully checked if this is supposed to be like this...

If I run the conversion in the tutorials E1 notebook, with definitions on "fairmat" branch, I get the following errors:
The data entry corresponding to /ENTRY[entry]/INSTRUMENT[instrument]/ELECTRONANALYSER[electronanalyser]/ENERGYDISPERSION[energydispersion]/energy_scan_mode is required and hasn't been supplied by the reader.
[info]: Path /ENTRY[entry]/PROCESS[process]/energy_calibration/coefficients not found. Skipping the entry.
The required group, /ENTRY[entry]/SAMPLE[sample]/bias/potentiostat, hasn't been supplied while its optional parent, /ENTRY[entry]/SAMPLE[sample]/bias, is supplied.

This is expected as for fairmat the config file does not match.

Still, the file is being created. If I run with "mpes-fixes" branch, I get:

The data entry corresponding to /ENTRY[entry]/INSTRUMENT[instrument]/ELECTRONANALYSER[electronanalyser]/ENERGYDISPERSION[energydispersion]/energy_scan_mode is required and hasn't been supplied by the reader.
[info]: Path /ENTRY[entry]/PROCESS[process]/energy_calibration/coefficients not found. Skipping the entry.

This is because it is named differently in metadata in the example file than the file we have in pynxtools. Changing this value "@attrs:metadata/energy_correction/calibration/coeffs" to this one "@attrs:metadata/energy_correction/calibration/coefficients" fixes it (L376 in config_file.json). The error about the energy_scan_mode is odd, because it should be set to optional in mpes-fixes and I also don't get this error. Can you try this one again?
Here is the associated PR with updated example files.

Sorry, I also should've mentioned that I did not yet test the examples with this. But I think E1 should be good now and I will check the error for the E2 as well.

domna · 2024-01-19T12:11:15Z

I think we should add a test that the writer does not try to create a link if the link target does not exist...

We check for that and abort if the target does not exist:

pynxtools/pynxtools/dataconverter/writer.py

Line 179 in 279ba0e

logger.warning("No path '%s' available to be linked.", path)

The problem in E2 is that it is trying to write an empty field, I think.
Does it work if you set the metadata manually w/o gathering it from the archive?

rettigl · 2024-01-19T14:36:45Z

The problem in E2 is that it is trying to write an empty field, I think.
Does it work if you set the metadata manually w/o gathering it from the archive?

I can try that. It's probably in the template at that point, but None. So this check should be included (that's what I meant).

domna · 2024-01-19T14:49:01Z

The problem in E2 is that it is trying to write an empty field, I think.
Does it work if you set the metadata manually w/o gathering it from the archive?

I can try that. It's probably in the template at that point, but None. So this check should be included (that's what I meant).

I already checked it. It also has problems when the metadata is set manually and I think it's because there are some additional keys in the associated eln file. I'm working on it rn.

Yes, with the latest changes everything is working for E2 :)

@rettigl please feel free to give it another check but it should be good to go then.

rettigl

Everything seems to be working now, also with the examples.

Definitions upgraded to latest mpes-refactor

9f8fe41

domna changed the title ~~MPES reader changes from the mpes refactoring~~ MPES reader changes from mpes refactoring Jan 8, 2024

Adapt vscode settings and update definitions, again

b86a5fe

domna marked this pull request as draft January 16, 2024 10:18

domna added 2 commits January 17, 2024 16:11

Updates definitions

781826b

Updates config file

26841e4

domna mentioned this pull request Jan 17, 2024

[Bug]: Fields are reported as missing even though they are present in the nexus file #212

Closed

domna added 3 commits January 17, 2024 16:56

Use nested config file structure

872fa05

flatten_json cleaned up

9a3246b

Updates definitions

4d469fa

domna requested a review from rettigl January 17, 2024 17:14

domna and others added 18 commits January 18, 2024 09:12

Renames DATA[data] to just data

6e56b02

Use beam_TYPE and source_TYPE

e72f25e

Add flag to reatin link dicts for flatten_json

fa4e27f

Properly resolve upper/lower case group fields

669dc52

Fixes a lookup error when keys have been filled in template with a di…

faa2792

…fferent name

Fixes the case where the reader sends back a template entry with the …

7514c1d

…concept name included

Correct coeffs path for raw data file

fc4b034

Fix path in data dict test

57a3900

Update mpes reference file

c9142e9

Updates reference nexus log

adb6926

Merge branch 'master' into mpes-reader-update

7c07f35

Fix nexus regression file

f890df0

Don't use removeprefix to support py3.8

d8a8b36

Move pressure_gauge to correct location

96e40a1

Corrects link to pressure_gauge

efb56e8

Updates reference mpes test log

5b45780

Updates path for eln mapping

fa1db7d

Fixes eln mapping

27fe1b8

domna requested review from sherjeelshabih and lukaspie January 18, 2024 17:03

Updates mpes reference file

eda5262

rettigl reviewed Jan 18, 2024

View reviewed changes

domna added 8 commits January 19, 2024 09:32

Removed applied fields in calibrations

9140ff3

Removed photon_energies from source

5ba4f66

Nest electronanalyser under instrument

3a3f86f

Use momentum_resolution and spatial_resolution as named concepts

31f71fe

Fix eln mapping for spatial and momentum resolution in electronanalyser

e780737

Updates mpes reference file

44d94e7

Renamings

689a0ee

Renamings in NXsample

116f099

domna mentioned this pull request Jan 19, 2024

Should all voltage settings be set with NXenvironment + NXsensor + NXactuator FAIRmat-NFDI/nexus_definitions#148

Open

sherjeelshabih reviewed Jan 19, 2024

View reviewed changes

pynxtools/dataconverter/helpers.py Outdated Show resolved Hide resolved

Properly deal with undocumented @Units as discussed w/ @sherjeelshabih

279ba0e

domna added 2 commits January 19, 2024 13:21

Updates mpes reference file

3afb459

Correctly map chemical formula

3b04d23

Adds additional mappings

73728f7

domna requested review from sherjeelshabih and rettigl January 19, 2024 16:08

rettigl approved these changes Jan 19, 2024

View reviewed changes

domna merged commit 2831461 into master Jan 22, 2024

domna deleted the mpes-reader-update branch January 22, 2024 08:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MPES reader cha 10000 nges from mpes refactoring #203

MPES reader changes from mpes refactoring #203

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!

MPES reader cha 10000 nges from mpes refactoring #203

MPES reader changes from mpes refactoring #203

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pull Request Test Coverage Report for Build 7585289570

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!