panosc-vinyl / mcstasscript Goto Github PK

When calling the backengine for the very first time, if the force_compile is False than the instrument is never generated and then it fails. If force_compile is False, a check of the existance of the instrument file should be performed to trigger generation and compilation at least once.

Example with the libpyvinyl instrument database:

from instrumentdatabaseapi import instrumentdatabaseapi as API
repo = API.Repository(local_repo=".")

instrument_name = "ThALES"
myinstrument = repo.load("ILL", instrument_name, "HEAD", "mcstas", dep=False)

myinstrument.calculators[myinstrument._calculator_name].settings(force_compile=False)
myinstrument.sim_neutrons(10000000)
myinstrument.run()

Allow for %include in trace. This inserts other code from a file into that spot.

Working on McStasMcXtrace/McCode#1429 I have realised that a good number of the McStas example instruments make use of DEPENDENCY, e.g. via MCPL_input.

DEPENDENCY is gathered from both instr and comps and provides compile-time info for e.g. library linking.

It would be nice if McStasScript offered a similar possibility, e.g. via the syntax instr.add_DEPENDENCY(string).

A single one-liner which should go directly after the DEFINE INSTRUMENT name(parameter-list) line, e.g. as visible in https://github.com/McStasMcXtrace/McCode/blob/mccode-3/mcstas-comps/examples/ESS_BEER_MCPL.instr

System to read existing instrument file and export a nicely formatted mcstasscript file.

The exception InputError is raised in functions.py and plotter.py, but was never defined.

Currently the standard output is not returned by the os.system call that runs the McStas simulation. This would be helpful in case of errors and for following the simulation progress.

Have an instrument which have this kind of code:

COMPONENT COPY(Guide_curved) = COPY(Guide_curved)()
AT (0, 0, 0.5) RELATIVE PREVIOUS
ROTATED (0,rot,0) RELATIVE PREVIOUS

COMPONENT COPY(Guide_curved) = COPY(Guide_curved)()
AT (0, 0, 0.5) RELATIVE PREVIOUS
ROTATED (0,rot,0) RELATIVE PREVIOUS
...

Reader raises error, which he shouldnt:
"NameError: Component name "COPY(Guide_curved)" used twice, McStas does not allow this. Rename or remove one instance of this name."

Hi I just realized that after having read a pre-existing instrument with reader.McStas_file the original file and the one generated by McStasScript gave different results. I found that the issue was due to one line skipped by the Reader.

Original:

  COMPONENT slab = Arm()
  AT (0, 0, apothem1-0.0005) RELATIVE target
  Group Cadmium
  EXTEND %{
  SCATTER; 
  hithole=0;//...otherwise 0
  %}

become:

  COMPONENT slab = Arm()
  AT (0, 0, apothem1-0.0005) RELATIVE target
  EXTEND %{
  SCATTER;
  hithole=0;
  %}

By substituting Group Cadmium with GROUP Cadmium the conversion happens smoothly. Probably the parser should be allowed to read non Capital command? The original is correctly executed by McStas after all.

I run the code with McStas 2.7 and the last release of McStasScript.
Hope this help.

Currently filenames need two levels of ", as in filename = ""PSD.dat"".

The type of the variable is known, so if "PSD.dat" is given, the extra layer could be added automatically. Then however it is not possible to distinguish between a string variable and a filename. McStasScript does however have a dict of all defined parameters, and can test if this would be allowed. The only potential problem is if a variable exists with the same exact name as what is entered, but it was meant as text.

It would be worthwhile to have this system to simplify common tasks.

Make a configuration file that contains path to McStas / mcrun and perhaps a line width for text wrapping in for example Jupiter notebook environments.

The following error raises when trying to dump a McStas instrument with some parameters defined as pint.Quantity:

_pickle.PicklingError: Can't pickle <class 'pint.quantity.build_quantity_class.<locals>.Quantity'>: it's not found as pint.quantity.build_quantity_class.<locals>.Quantity

How to reproduce the issue:

git clone -b thales [email protected]:PaNOSC-ViNYL/instrument_database.git
git checkout 3f11a9f48e0247b514d16bc3ee7eb937a638d741
cd instrument_database/
pip install -e instrumentDataBaseAPI/

from instrumentdatabaseapi import instrumentdatabaseapi as API

repo = API.Repository(local_repo=".")
myinstrument = repo.load("ILL", "ThALES", "HEAD", "mcstas", dep=False)

import pint
ureg = pint.get_application_registry()

a2 = myinstrument.parameters["ThALES"]["a2"]
energy = 4.48
dE = 0.05
a2.energy = energy * ureg.meV
myinstrument.run()
myThALES = myinstrument.calculators["ThALES"]
myThALES.dump("/tmp/myThALES.py")

When adding a component like so:

slit = Instr.add_component("slit", "Slit")
slit.set_AT([0,0,1], RELATIVE="source")

The slit will be written:
COMPONENT slit = Slit()
AT (0, 0, 1) RELATIVE source
ROTATED (0, 0, 0) ABSOLUTE

Which is probably not what the user wanted, as it should probably have the same rotation as the source. A simple fix would be not to write rotation unless it has been specified.

System for adding a already defined part to a instrument, for example a cryostat. This could be accomplished as a merge operation of two instruments or a system for a subassembly.

There is currently no check of the given instrument parameters in relation to the known instrument parameters.

The McStas_instr class should have a method for displaying instrument parameters along with their default values and comments.

Currently the name_search function and similar finds data with the component name given, but could find it using data file name if it is unsuccessful.

Find a way to define default plot options of the data to be recorded by a monitor when that monitor is defined. This way code pertaining to that monitor is kept in one place, and it can be overwritten later if preferred.

Write a warning to the user with information on how to set parameters.

The SPLIT keyword is not supported, but should be added to the component class.

A boolean flag to allow requesting openacc via https://mads-bertelsen.github.io/_autosummary/mcstasscript.helper.managed_mcrun.ManagedMcrun.html?highlight=mpi#mcstasscript.helper.managed_mcrun.ManagedMcrun.mpi

When setting the relative keyword for a component, one has to refer to the name it was given in McStas. Could allow passing an component object and grab its name.

When running a simulation that has multiple MCPL_output components, a certain branch in McCode_instr.__add_mcpl_to_output() is triggered which attempts to add information about the number of the MCPL file to the its key.

At the moment, the number of mcpl files num_mcpl_files is converted to a string and supposed to be concatenated with mcpl_file, which is of type pyvinylMCPLData, causing a

TypeError: unsupported operand type(s) for +: 'pyvinylMCPLData' and 'str'

Maybe the key should be constructed like:

mcpl_file.key = mcpl_file.key + str(num_mcpl_files)

?

I ran into two issues in the show_instrument() functionality of macstasscript (version "0.0.55").

1. Windows 10
   show_instrument() only works if one hacks the call: to mcdisplay-webgl i.e without path and " " (see below)
   It fails otherwise. I think it is somehow caused by jupyter lab running in the dedicated mcstas-shell

if is_notebook and executable == "mcdisplay-webgl" and not new_tab:
            options += "--nobrowse "

        full_command = ('"' + bin_path + executable + '" '
                        + dir_control
                        + options
                        + instr_path
                        + " " + parameter_string)
      
        new_full_command = (executable + ' '
                        + dir_control
                        + options
                        + instr_path
                        + " " + parameter_string)
       

        process = subprocess.run(new_full_command, shell=True,
                                 stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE,
                                 universal_newlines=True,
                                 cwd=self.input_path)

2. Windows and MacOs:
   I get only the (empty) output in the notebook cell:
   

   new_tab=True opens a new browser tab and works as intended
   IFrame() seems to find the html but not the js files

Many of the names given by the user have limitations set by McStas and c. For example parameters can not have spaces in them or start with a number. Raise an error when the user attempts to do so.

Using the ms.McStas_file method on the ILL_H22 template instrument works fine, but the naming of the components is off.

Some of the guides are copies of another guide, but instead of naming them with an index, say H22_6_0 and then H22_6_1, it names them as H22_6_0_copy and then H22_6_0_copy_copy, and on it continues for as many copies of the guide you make. In the code it seems as if indexing is supposed to be added after the "_copy" but this is not what happens.

With the recent "embedded" NeXus support in McStas 3.3 it would be nice to be able to request output in NeXus format also from McStasScript, including the IDF generation.

Is it somehow possible to add "further parameters" for running mcrun? Either using instr.settings() or along the lines of
data = instr.backengine('--format=NeXus --IDF') ?

If not, please consider this issue a feature request... ;-)

The AT or ROTATED vectors need to have length 3, which is not checked. The user should be alerted with a reasonable error message at the component, yet currently the code only fails when trying to write the instrument file.

Description

When reading a component file that includes a SETTING PARAMETERS value

int readout_type = 0x34,

an error is thrown at the following line due to int(str) only accepting base 10 representations of integers.

Expectation

Since the literal value is copied from a component file to the C source file, it is legal McStas syntax to specify integer numbers in binary, octal, decimal, or hexidecimal bases (e.g., 0b110100, 064, 52, and 0x34 are all valid syntax and result in the same integer).

Without explicit and clear warning otherwise, it is expected that all valid McStas components and instruments should also work with McStasScript. In the current state a user of McStasScript could attempt to use a component which they did not author and encounter this problem which they then do not know how to fix.

Possible solutions

1. Require that all integers read from files be expressed in base 10. Add warnings to the McStasScript documentation, and encapsulate the call to int(par_value) in a try block with a helpful error message upon failure.
2. In the documentation for int(), it is noted that automatic base determination is possible. Use, e.g., int(par_value, 0), to automatically distinguish binary, decimal, and hexadecimal values.
3. Check and set the expected base from the contents of par_value.

Shortcomings

Option 1 has the significant drawback that an octal number will be silently converted to decimal, and for option 2 the conversion will fail with a ValueError.
Option 3 will require extra logic with the potential for extra bugs.

Desired solution

With full support for C integer literals and no silent value conversions only the third option is acceptable.

With the following possible implementation to identify the base of a C integer literal

def c_integer_literal_base(s: str) -> int:
     if len(s) == 0:
         return 0
     if '0' != s[0]:
         return 10
     if 'x' in s:
         return 16
     if 'b' in s:
         return 2
     return 8

the decoding step on line 388 could be changed to par_value = int(par_value, c_integer_literal_base(par_value))

Currently initialization of parameters is done in the declare section of the McStas file, but in the upcoming McStas 3.0, this is not likely to be allowed. Move parameter initialization to the initialize section.

The McXtrace software has a special sub-category for the so-called "AstroX" components for X-ray telescopes.

Please add 'astrox' to the relevant search path in mcstasscript/helper/component_reader

When initializing arrays there is no check for matching dimensions.

McStas has mcdisplay that shows the instrument. This should somehow be implemented in mcstasscript. A call to mcdisplay, either directly plotting to notebook or plot the resulting data.

Currently it is only possible to declare one dimensional arrays and provide these with initial values. This is to be expanded to arbitrary dimensions.

After a McStas run, the data is read from the generated folder without any reference to the order of the monitors in the instrument file. No reference to the component index is contained in these files. The mccode.sim plotting file contains this information, but has been ignored so far.

Should restructure the read system to look for mccode.sim and then find the related files, which would also allow errors if a file is not found.

Currently the code works well for scripting, but can be problematic in a Jupyter Notebook as problems occur when a cell is executed several times. For example extra components can be added, or errors can occur if components with the same name is added twice.

This has to be addressed in order to improve the user experience