Added an additional text box to input the model description for ORSO file formats

[RabiyaF]

Description of work

Closes #40954.
Adds an additional textbox in the Save tab of the ISIS Reflectometry Interface so that the user can provide a custom model description.

To test:

1. Run the following lines of code to create a workspace

ws = CreateSampleWorkspace(XUnit="MomentumTransfer", NumBanks=1, BankPixelWidth=1)
AddSampleLog(Workspace=ws, LogName='rb_proposal', LogText='1234', LogType='Number')

2. Open the ISIS Reflectometry interface and navigate to the Save tab.
3. Select the ws workspace.
4. Select either ORSO Ascii (*.ort) or ORSO Nexus (*.orb).
5. Verify that the additional text box is visible.
6. Enter air | Ni 100 | SiO2 0.5 | Si into the model description textbox.
7. Click Save.
8. Verify that these two lines are visible in the file when a string is provided

9. Remove the air | Ni 100 | SiO2 0.5 | Si from the textbox and click Save.
10. Verify that the lines are no longer visible in the file.

11. Now add an incorrect string into the textbox. For example, air | Ni 100 | SiO2 0.5 | 02 or air | 25 [Si 7 | Fe 7 ] | Si and click Save.
12. Verify that the user is informed of an error in the ORSO model description.

---

Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

• Is the code of an acceptable quality? (Code standards/GUI standards)
• Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
• Are appropriately scoped unit and/or system tests provided?
• Do the release notes conform to the guidelines and describe the changes appropriately?
• Has the relevant (user and developer) documentation been added/updated?
• If the PR author isn’t in the mantid-developers or mantid-contributors teams, add a review comment rerun ci to authorize/rerun the CI

Gatekeeper

As per the gatekeeping guidelines:

• Has a thorough first line review been conducted, including functional testing?
• At a high-level, is the code quality sufficient?
• Are the base, milestone and labels correct?

[jhaigh0]

This works as described and the code is sensible and tested 👍

My only question is, does the model string need to be validated at all? At the moment it will just write what ever string you enter to the file, if you enter an invalid model or maybe a string with : in it (or any other special character for the ORSO format) would it stop the saved file from being read back in? (to Mantid or any other program reading ORSO files)

If there is a strict format for this model string then I think it should be checked, but I'm not familiar with the ORSO format

[MialLewis]

This works as described and the code is sensible and tested 👍

My only question is, does the model string need to be validated at all? At the moment it will just write what ever string you enter to the file, if you enter an invalid model or maybe a string with : in it (or any other special character for the ORSO format) would it stop the saved file from being read back in? (to Mantid or any other program reading ORSO files)

If there is a strict format for this model string then I think it should be checked, but I'm not familiar with the ORSO format

The rules governing the simple single line model as implemented here are detailed:
https://www.reflectometry.org/projects/simple_model#stack

As you can see, there is scope for increased complexity over multiple lines but this requirement has not yet been expressed by our users.

Validation would be great; but we should avoid implementing ourselves. The orsopy library SampleModel object seems to have a resolve_stack method which has some validation. I think we should use this. It seems to check the formatting implicitly, as it attempts to parse the string for the chemical formula, then looks up the chemical formula against a list.

Tests for this validation would also be good.

[RabiyaF]

I spoke with Stephen (The RasCAL team lead) and he said that they use the method resolve_to_layers (it calls resolve_stack) in writing their tests. However, he warned that the database that verifies the chemical formula can sometimes be unavaliable. So they have had to mark their tests with the skip decorator. At the moment the database is working and that is why I have added the tests. But it is something that they want you to be aware of. You will also notice that the save operation is now slower because of the validation and database look up.

[jhaigh0]

Thanks for adding the validation. I think given the warning we've been given we need to be more careful about the validation method.

If, in the case where the database fails to connect, that method raises an exception, then we can simply catch it as I described in my comment.
If, when it fails, the method just hangs, then I think we need to guard against this.

Here is some copilot code showing how to use concurrent.futures to run something for a maximum of n seconds before quitting and continuing.

import concurrent.futures
from typing import Callable, Any

def run_with_timeout(func: Callable, timeout: float, *args, **kwargs) -> Any:
    """
    Run `func` with the given timeout.
    
    If `func` does not return within `timeout`, return None
    and print a warning.
    """
    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor:
        future = executor.submit(func, *args, **kwargs)

        try:
            return future.result(timeout=timeout)

        except concurrent.futures.TimeoutError:
            print(f"⚠️ Function '{func.__name__}' timed out after {timeout} seconds")
            return None

[jhaigh0]

Since you mentioned this method can fail if it fails to connect to the database, if in that case it throws an exception, it will be caught here and prevent the save. That might be annoying if someone's computer is failing to connect.
I think you should catch the specific exception raised for a connection failure (hopefully that is documented somewhere) and in that case just log a warning saying that the validation wasn't available.

I also think it would be neater if you could just print an error message to the log (and then exit early) rather than raise an exception which fills the log with a trace back. Maybe whoever calls this method could catch a ValueError?