Add YAML file specification

[RastislavTuranyi]

Adds a detailed specification for the YAML data files to the documentation.

Additionally, adds a script that validates a given YAML file - that it is structured correctly as well as that it can be run with ResINS. It currently raises an exception on the first issue it finds, but maybe it should instead log all of them into a given output? On that note, it seems like the default values of e_init and chopper_frequency for the ARCS instrument result in NoTransmissionError, so I suppose we should change the defaults, not sure whether to do that here or separately though.

[ajjackson]

At the moment the validator seems to be failing on the included .yaml data, both before and after rebase.

Inclined to rebase, add it to CI, and fix before merging.

[RastislavTuranyi]

I think the main reason for that is #50 - not sure if you want to do that in here or in a separate PR + will likely need input from instrument scientists

[ajjackson]

I see! Changing the default e_init for chopper instruments should not be too controversial as in practice this will nearly always be provided.

[ajjackson]

I can't tweak IDEAL2D YAML to pass validation because it isn't implemented. Shall we delete this for now, and restore with an implementation later?

[RastislavTuranyi]

I think that one is basically a holdover from copying everything from AbINS and hasn't been changed since the beginning - I'd say delete it and if we want the functionality, it could be implemented in #45 or similar

[ajjackson]

For TOSCA we are getting the validation error

 INFO:root:src/resolution_functions/instrument_data/tosca.yaml
Traceback (most recent call last):
  File "/home/runner/work/resins/resins/dev/validate_data_file.py", line 291, in validate_with_resins
    instrument.get_model_data(model_name=model, **kwargs)
  File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/resolution_functions/instrument.py", line 401, in get_model_data
    out, _ = self._get_model_data(model_name, **kwargs)
  File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/resolution_functions/instrument.py", line 446, in _get_model_data
    model = model_class.data_class(function=model['function'],
TypeError: ToscaBookModelData.__init__() got an unexpected keyword argument 'final_neutron_energy'

This seems to be caused by a conflict between the declared constants and the (currently known-broken) "book" model. But:

• I'm not sure every parameter in "constants" needs to be known to every model, because we can have multiple models for the same instrument and they won't all need the same info
• This is a "runtime-ish" problem, checking that the data matches the function signature. A formal schema-checking system like json-scheme won't provide that, and would just check that the general data structure is correct. Should this validator be doing both checks, or should they be separate?

update A-ha, I see we can use the --disable-yaml and --disable-resins arguments for this. Will tweak the CI.

[RastislavTuranyi]

Seems like I forgot to remove the constants for TFXA and TOSCA1 when refactoring (TOSCA does not have final_neutron_energy) and the same goes for scattering_angle and energy_bin_width. change_average_bragg_angle_graphite also seems to be missing from TFXA, not sure why though (the blue book might have been missing a value, or i just forgot, in which case it can be computed using the calculate_tosca_book_delta_theta_b function in developer_utils.py)

[ajjackson]

Ah, so the problem is that

• extra information is in constants, and
• the book model pulls them in with parameters: *tosca1_constants

Whereas for the Abins TOSCA model, parameters is defined in-place without using *tosca_constants so it doesn't matter that they aren't relevant.

[RastislavTuranyi]

TOSCA has only the common stuff in constants - everything not shared is in the parameters for the appropriate model - i was testing with it so everything there is correctly formatted and fairly nicely organised. Tbh, since both TOSCA1 and TFXA only have one model each, there is no real reason to use YAML magic - everything can be placed directly into its final place.

[RastislavTuranyi]

Is this value supposed to be the same for TFXA and TOSCA1?

[ajjackson]

Better check the book when I have access to it tomorrow! But I think it's the same analyzer technology?

[RastislavTuranyi]

I think some of the underlying constants that go into the calculation might have changed between the two

[ajjackson]

Right you are! With this script:

from dataclasses import dataclass
import math

@dataclass
class BookModelData:
    d_i: float           # Primary flightpath (m)
    d_f: float           # Average secondary flightpath (m)
    E_f: float           # Average final energy (meV)
    θ_B: float           # Average Bragg angle for graphite (°)
    A: float             # ISIS water moderator constant (μs)
    Δt_ch: float         # Time channel uncertainty (μs)
    d_s: float           # Sample thickness (m)
    d_g: float           # Graphite thickness (m)
    d_d: float           # Detector thickness (m)
    Δd_i: float          # Uncertainty in primary flightpath (m)
    w_s: float           # Sample width (m)
    w_d: float           # Detector width (m)
    η_g: float = 2.5     # Mosaic of the graphite analyser (°)
    ħ2: float = 4.18019  # Conversion factor (meV Å)

def cot(x: float) -> float:
    return 1 / math.tan(x)


def get_Δθ_B(data: BookModelData) -> float:
    α_2 = data.w_s / 2 * (1 + cot(data.θ_B * math.pi / 180.)**2) / data.d_g
    α_3 = data.w_d / 2 * (1 + cot(data.θ_B * math.pi / 180.)**2) / data.d_g

    Δθ_B = (math.sqrt(α_2**2 * α_3**2 + α_2**2 * data.η_g**2 + α_3**2 * data.η_g**2)
            / math.sqrt(α_2**2 + α_3**2 + (2 * data.η_g)**2))
    return Δθ_B

txfa_data = BookModelData(12.13, 0.671, 2.909, 43.0, 44., 2., 0.002, 0.002, 0.006, 0.0021, 0.02, 0.012)
tosca1_data = BookModelData(12.264, 0.7456, 3.51, 46.03, 44., 2., 0.002, 0.002, 0.0025, 0.0021, 0.020, 0.012)

print("TXFA Δθ_B: ", get_Δθ_B(txfa_data))
print("TOSCA1 Δθ_B: ", get_Δθ_B(tosca1_data))

I get

TXFA Δθ_B:  5.637684123246608
TOSCA1 Δθ_B:  5.0814852427945665

Is there somewhere more responsible I should store that?

[RastislavTuranyi]

As mentioned earlier, there's developer_utils.py, though it seems like you have made a more elaborate version

[ajjackson]

Hmm, PANTHER is failing because e_init is a mandatory argument and the validator doesn't supply anything.

If we want this validator to pass all our YAML files, we can't have mandatory arguments without defaults.

Or we need to provide the validation defaults somewhere else.

[ajjackson]

Looks like we have green checkmark.

We're still in alpha and the spec can change if necessary. The docs and validator from this PR will help guide and implement that.

Apart from that TXFA parameter, are there any other blockers to this PR?

[RastislavTuranyi]

If we're looking to rewrite the entire script anyway, there's no point harping on the details, so, yes, i thing this could go ahead.

[oerc0122]

Does the validate want to be added as a script in the pyproject.toml? Or are we keeping it for debugging?

[RastislavTuranyi]

I think we want to make it available to people who design their own models, which can be done with ResINS only pip-installed (without having to pull the entire repo), so it's probably a good idea. Maybe it could be hidden behind a flag? e.g. pip install resins[dev] would also install the script

[ajjackson]

It would be good if it can be hidden in a dev tools option: I don't think it should be pip-installed for anyone who pulled in ResINS as a dependency. But I don't think pyproject.toml makes that easy?

Probably the official technical solution is a separate repository as an optional dependency...

[oerc0122]

I don't think it's possible with the current pyproject spec (using setuptools) it's one of the issues we're having with castep outputs

You can, however, set up an optional dependency as a git+... we could also make it a multi-project repo using find=...

[ajjackson]

I do expect that some CLI tools will be added for general users, though. In which case it is less annoying for the validator to be hidden behind a general resins program entry point.

Let's leave it uninstalled for now and revisit when other CLI tools are added.

[ajjackson]

I think this already does a lot of good. We are still in alpha and can change details if necessary; let's reduce the PR/merge backlog first.