Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compile_and_run convenience function #54

Open
tkittel opened this issue Aug 27, 2024 · 24 comments
Open

Add compile_and_run convenience function #54

tkittel opened this issue Aug 27, 2024 · 24 comments

Comments

@tkittel
Copy link

tkittel commented Aug 27, 2024

As we discussed, it would be great to be essentially able to parse, compile, and run a sample .instr file in a few lines of code (i.e. in a notebook as part of a tutorial). The function compile_and_run that I was kindly pointed at in https://github.com/McStasMcXtrace/mccode-antlr/blob/main/tests/runtime/compiled.py#L85 looks like something that would be useful as a general utility function.

But possibly taking the directory as an argument, instead of the TemporaryDirectory thing (which the user could do in their calling code if needed)?

@g5t
Copy link
Collaborator

g5t commented Aug 27, 2024

I guess you'd prefer a single function, but currently two functions can compile and run a parsed Instr object -- mccode_antlr.run.mccode_compile and mccode_antlr.run.mccode_run_compied.

A possible way to use the functions is in the updated version of the compile_and_run test utility you linked to before:

def compile_and_run(instr, parameters, run=True, dump_source=True, target: dict | None = None, config: dict | None = None):
from pathlib import Path
from tempfile import TemporaryDirectory
from mccode_antlr.translators.target import MCSTAS_GENERATOR
from mccode_antlr.run import mccode_compile, mccode_run_compiled
kwargs = dict(generator=MCSTAS_GENERATOR, target=target, config=config, dump_source=dump_source)
with TemporaryDirectory() as directory:
binary, target = mccode_compile(instr, directory, **kwargs)
# The runtime output directory used *can not* exist for McStas/McXtrace to work properly.
# So find a name inside this directory that doesn't exist (any name should work)
return mccode_run_compiled(binary, target, Path(directory).joinpath('t'), parameters) if run else (None, None)

Would this work for you? If so, I can pull together a release for PyPI.

@tkittel
Copy link
Author

tkittel commented Aug 27, 2024

For sure, that version of compile_and_run is much shorter, so for the workshop next week it should be good enough!

But thinking aloud... perhaps I would still love to be spoiled a bit more at some point in the future, i.e. have a "mcstas_run" function, which would automatically use the MCSTAS generator, and accept an "instrument" (i.e. a string, file, or an already-parsed instrument), and also accept a working directory argument (since in general the results should probably not be cleaned up before the function exits... i guess).

So Ideally I would love to have code like:

from mccode_antler.whatever import mcstas_run
myinstr="""
BLA SOME INSTRUMENT CODE HERE
"""
mcstas_run(myinstr,'./somedir')
#or:
with TemporaryDirectory() as d:
    mcstas_run(myinstr,d.joinpath('somedir'))
    #analyse output here

Does that make sense? However, there might be stuff I am not thinking about and so forth, so perhaps it is best that for the short term I simply try and use the few functions you already provide?

@g5t
Copy link
Collaborator

g5t commented Aug 27, 2024

If you can come up with a sensible way to handle directories, and want to write the logic to take 3+ types of input as a positional argument, this could be a useful addition.

The directory scheme should probably match (or at least resemble) the McCode behavior, where the working directory is always the current directory, the binary ends up named after the instrument file, and the runtime output artifacts end up in a newly-created folder either specified as a command line option or named after the instrument file and current time.

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Sounds good to me, let us discuss it in person next time we have a chance!

In the meantime, for my workshop, I was trying to add a small utility function at the top of my google colab notebooks:

def mcstas_run( instr, rundir = None ):
    import pathlib
    import tempfile
    from mccode_antlr.translators.target import MCSTAS_GENERATOR as GEN 
    from mccode_antlr.run import mccode_compile, mccode_run_compiled 
    rundir = pathlib.Path( tempfile.mkdtemp('mcstas_rundir_')
                           if rundir is None else rundir )
    if not rundir.is_dir():
        raise FileNotFoundError("Directory not found: %s"%rundir_base)
    binary, target = mccode_compile(instr, rundir, generator=GEN)
    mccode_run_compiled(binary, target, rundir)
    return rundir

However, I didn't get to really test it yet since I get the error:

Downloading antlr4-4.13.2-complete.jar
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-4-372250295857> in <cell line: 1>()
----> 1 outdir = mcstas_run(my_instr)

<ipython-input-2-40ce4132f61d> in mcstas_run(instr, rundir)
      3     import tempfile
      4     from mccode_antlr.translators.target import MCSTAS_GENERATOR as GEN
----> 5     from mccode_antlr.run import mccode_compile, mccode_run_compiled
      6     rundir = pathlib.Path( tempfile.mkdtemp('mcstas_rundir_')
      7                            if rundir is None else rundir )

ModuleNotFoundError: No module named 'mccode_antlr.run'

Does that mean that I have to wait for you to make a new PyPI package first?

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Actually, I guess I can just pip install straight from your repo to try it out.

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Ok, this seemed to work:

%pip install git+https://github.com/McStasMcXtrace/mccode-antlr@issue-54-run

I will try it out now.

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Ok, so this went better. I had issues with the SHELL statement introduced in McStas 3.3, but without that and the updated function here I was able to proceed a bit further:

def mcstas_run( instr, rundir = None ):
    if hasattr(instr,'__fspath__') or not hasattr(instr,'name'):
        from mccode_antlr.loader import parse_mcstas_instr
        instr = parse_mcstas_instr(instr)
    import pathlib
    import tempfile
    from mccode_antlr.translators.target import MCSTAS_GENERATOR as GEN 
    from mccode_antlr.run import mccode_compile, mccode_run_compiled 
    rundir = pathlib.Path( tempfile.mkdtemp('mcstas_rundir_')
                           if rundir is None else rundir )
    if not rundir.is_dir():
        raise FileNotFoundError("Directory not found: %s"%rundir_base)
    binary, target = mccode_compile(instr, rundir, generator=GEN)
    mccode_run_compiled(binary, target, rundir)
    return rundir

Next problem is that it seems to hang after the following output. Do I need to somehow pass in a different number of sr neutrons? Actually, how DO I pass in parameters? Do you have an example?

...
Downloading file 'common/lib/share/metadata-r.c' from 'https://github.com/McStasMcXtrace/McCode/raw/main/common/lib/share/metadata-r.c' to '/root/.cache/libc'.
No initialization present?

-----------------------------------------------------------

Generating single GPU kernel or single CPU section layout:

-----------------------------------------------------------

Generating GPU/CPU -DFUNNEL layout:
-> GPU kernel from component myMat_ncrystal_proc
-> GPU kernel from component myMat
-> GPU kernel from component init
-> GPU kernel from component origin
-> GPU kernel from component source
-> GPU kernel from component powder_sample
Component master2 is NOACC, CPUONLY=False
->FUNNEL mode enabled, SPLIT within buffer.
-> GPU kernel from component master2
-> GPU kernel from component powder_pattern_detc
-> GPU kernel from component stop

-----------------------------------------------------------
Downloading file 'common/lib/share/mccode_main.c' from 'https://github.com/McStasMcXtrace/McCode/raw/main/common/lib/share/mccode_main.c' to '/root/.cache/libc'.
2024-08-28 08:34:39.551 | INFO     | mccode_antlr.compiler.c:compile_instrument:137 - Source written in example2.c

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

For reference, this was my instrument file (actually, in a string):

DEFINE INSTRUMENT example2()
TRACE

/*
   The following code was auto generated by NCrystal v3.9.4 via Python:

     NCrystal.mcstasutils.cfgstr_2_union_instrument_code(
         cfgstr = 'phases<0.01*void.ncmat&0.99*Al_sg225.ncmat;temp=200K>',
         name = 'myMat' )

   Please rerun in case of major changes to input data or NCrystal.
*/

COMPONENT myMat_ncrystal_proc = NCrystal_process(
    cfg = "phases<0.01*void.ncmat&0.99*Al_sg225.ncmat;temp=200>" )
AT (0,0,0) ABSOLUTE

COMPONENT myMat = Union_make_material(
    process_string = "myMat_ncrystal_proc",
    my_absorption = 1.37745435679474 )
AT (0,0,0) ABSOLUTE

/* End of auto generated code from NCrystal v3.9.4. */

COMPONENT init = Union_init()
AT (0,0,0) ABSOLUTE
COMPONENT origin = Progress_bar()
  AT (0, 0, 0) RELATIVE ABSOLUTE
COMPONENT source =   Source_div(lambda0=1.539739, dlambda=0.01, xwidth=0.001, yheight=0.001, focus_aw=1, focus_ah=1)
  AT (0, 0, 0.3) RELATIVE origin
COMPONENT powder_sample = Union_cylinder(yheight=0.01, radius=0.01, priority=1, material_string="myMat")
AT (0, 0, 1) RELATIVE origin
COMPONENT master2 = Union_master()
AT (0, 0, 0) RELATIVE powder_sample
COMPONENT powder_pattern_detc = Monitor_nD(
    options = "banana, angle limits=[10 170], bins=500",
    radius = 0.05, yheight = 0.1)
  AT (0, 0, 0) RELATIVE powder_sample
COMPONENT stop = Union_stop()
AT (0,0,0) ABSOLUTE
END

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

It's good that installing from the git branch of the repository works 😆


Actually, how DO I pass in parameters? Do you have an example?

The binary runtime input parameters should be specified as a string, as in the test helper function

mccode_run_compiled(binary, target, Path(directory).joinpath('t'), parameters)

I'm a bit surprised that your call to mccode_run_compiled with only three positional inputs worked at all; I would have expected a TypeError for the missing parameter.

At the moment it's up to you to know what parameters your binary will accept, and the standard McCode compiled instrument parameters are of course allowed.

Note

You probably don't want to specify --dir {directory} in the parameters string, since that is added for you from the third positional input of the call to mccode_run_compiled

Warning

Your snippet will raise an error because you're passing the working directory rundir to mccode_run_compiled and McStas refuses to output simulation result artifacts into an existing directory.

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

@tkittel You should now be able to install mccode-antlr v0.7.0 directly via pip to get the new functionality.

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Great! I will most likely have to continue my attempts tomorrow.

But when you say that McStas refuses something, isn't that in your code now? Or is it in the c-files?

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

So I still can't get this to run. It seems to hang during compilation (or just takes longer than my attention span). For reference, classic mcrun parses, compiles, and runs in <1sec.

Here is a standalone script showing my problem:

#!/usr/bin/env python3

#Needs "pip install mccode_antlr ncrystal"

_test_instr="""
DEFINE INSTRUMENT example2()
TRACE

COMPONENT origin = Progress_bar()
AT (0, 0, 0) RELATIVE ABSOLUTE

COMPONENT source =   Source_div(lambda0=1.539739, dlambda=0.01, xwidth=0.001, yheight=0.001, focus_aw=1, focus_ah=1)
AT (0, 0, 0.3) RELATIVE origin

COMPONENT mysample = NCrystal_sample(cfg="Al2O3_sg167_Corundum.ncmat",yheight=0.01,radius=0.01)
AT (0, 0, 0) RELATIVE PREVIOUS

COMPONENT powder_pattern_detc = Monitor_nD(
    options = "banana, angle limits=[10 170], bins=500",
    radius = 0.05, yheight = 0.1)
AT (0, 0, 0) RELATIVE PREVIOUS

END
"""

def mcstas_run( instr, rundir = None ):
    if hasattr(instr,'__fspath__') or not hasattr(instr,'name'):
        from mccode_antlr.loader import parse_mcstas_instr
        instr = parse_mcstas_instr(instr)
    import pathlib
    import tempfile
    from mccode_antlr.translators.target import MCSTAS_GENERATOR as GEN 
    from mccode_antlr.run import mccode_compile, mccode_run_compiled 
    rundir = pathlib.Path( tempfile.mkdtemp('mcstas_rundir_')
                           if rundir is None else rundir )
    if not rundir.is_dir():
        raise FileNotFoundError("Directory not found: %s"%rundir)
    for i in range(1,1000000):
        #Quick and dirty solution, to be changed:
        actual_rd = rundir / f'mcstasrun_{i}'
        if not actual_rd.exists():
            break
    binary, target = mccode_compile(instr, rundir, generator=GEN)
    mccode_run_compiled(binary, target, actual_rd, parameters='-n 1000')
    return actual_rd

def main():
    mcstas_run( _test_instr, './lala' )

if __name__=='__main__':
    main()

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Hmm... investigating a bit further, it seems to only hang when NCrystal_sample is present.

Could it be the FUNNEL (non-gpu) mode?

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

But when you say that McStas refuses something, isn't that in your code now? Or is it in the c-files?

The runtime refuses, see

https://github.com/McStasMcXtrace/McCode/blob/584eb8d56e493b29c856a4bfb3273de181f4cc77/common/lib/share/mccode-r.c#L2393-L2396

    if(mkdir(dirname, 0777)) {
#ifndef DANSE
      fprintf(stderr, "Error: unable to create directory '%s' (mcuse_dir)\n", dir);
      fprintf(stderr, "(Maybe the directory already exists?)\n");

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

I haven't used NCrystal_sample -- does it do any @include or %include-ing (or whatever the special McCode include keyword is)?

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

I thought maybe it could be stuck working-out circular includes of, e.g., read_table-lib, interoff-lib, etc.;
but that happens before the snippet you included above so that isn't the problem.

The DEPENDENCY injection can work (I copied your implementation for Readout) -- is ncrystal-config available in your environment?

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Yes, and I see the compilation command picks up the correct flags from it...

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

It's somehow related to libNCrystal.so.3.9.5 and reading the source contents from stdin.

Using the example2.c dumped by the hanging code, compare the difference between:

cc -g -O2 -D_POSIX_C_SOURCE -std=c99 example2.c -lm -Wl,-rpath,$(ncrystal-config --show libdir) $(ncrystal-config --show libpath) -I$(ncrystal-config --show includedir) -DFUNNEL

and

cc -g -O2 -D_POSIX_C_SOURCE -std=c99 -x c - -lm -Wl,-rpath,$(ncrystal-config --show libdir) $(ncrystal-config --show libpath) -I$(ncrystal-config --show includedir) -DFUNNEL < example2.c

The first works fine, the second will fill your terminal with all sorts of warnings about stray undefined escape sequences and null characters in what looks like a binary blob in your shared library.

I don't know whether using a stdin pipe for the source code is prone to this sort of problem, or if there is something special about the NCrystal shared library. Have you seen this before?


It is somehow independent of whether the library is even used. With a very simple main.c

#include <stdio.h>
int main(){
    printf("Hello world\n");
    return 0;
}
cc -g -O2 -D_POSIX_C_SOURCE -std=c99 -x c - -lm -Wl,-rpath,$(ncrystal-config --show libdir) $(ncrystal-config --show libpath) -I$(ncrystal-config --show includedir) -DFUNNEL < main.c

produces exactly the same output to stderr

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

The use of ncrystal-config obfuscates the issue a bit, if we pick some concrete values:

$ ncrystal-config --show libdir
./lib
$ ncrystal-config --show libpath
./lib/libNCrystal.so.3.9.5
$ ncrystal-config --show includedir
./include

and dropping the compiler flags for the moment, the compilation command looks like

$ cc ... -x c - -lm -Wl,-rpath,./lib ./lib/libNCrystal.so.3.9.5 -I./include -DFUNNEL < main.c

We can, for the moment, behave like gcc and ignore any -l* or -Wl,* options

$ cc ... -x c - ./lib/libNCrystal.so.3.9.5 -I./include -DFUNNEL < main.c

so its parsing the shared object file because it appears like a source on the command line.

Usefully, it's possible to specify the location -L and exact library filename -l: -- at least for gcc, and hopefully for the other target compilers -- and ncrystal-config --show libname already exists, so the command can be changed to

$ cc ... -x c - -lm -Wl,-rpath,./lib -L./lib -l:libNCrystal.so.3.9.5 -I./include -DFUNNEL < main.c

and then it compiles as expected without any attempted parsing of the shared library as code.

Is there a reason that the NCrystal DEPENDENCY injection doesn't use -LCMD(ncrystal-config --show libdir) -l:CMD(ncrystal-config --show libname) (or -LCMD(ncrystal-config --show libdir) -lNCrystal, since the library name isn't changing)?

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

Ah, so it was compiling the shared library! Sick that it didn't just error out :-)

So I am a bit confused about the relative paths you get from ncrystal-config, since I could have sworn they should be printed with absolute values. But apart from that I get your point. So basically it boils down to how mcstas components only have one DEPENDENCY line to be shared between compilation and link steps. And then I guess mccode-antlr and classic mcrun somehow orders the flags differently. Ok, thanks for figuring that out.

Now, on one hand should you not try to mimic the classic mccode order? But on the other hand, I agree we should try to make it more robust in any case... So -LCMD(ncrystal-config --show libdir) -l:CMD(ncrystal-config --show libname) would work (not -lNCrystal since the library name actually DOES change sometimes). I will make a PR for upstream mccode after investigating a bit.

@tkittel
Copy link
Author

tkittel commented Aug 28, 2024

So for testing, how does I tell mccode_antlr to use a locally edited NCrystal_sample.comp ?

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

So for testing, how does I tell mccode_antlr to use a locally edited NCrystal_sample.comp ?

Drop it in the same directory where you call mcstas-antlr, or since you're doing it from within Python, define a LocalRegistry that points to the folder containing it then pass it to the parser.
For inspiration, see how the command line interface does it:

def mccode(flavor: str, registry: Registry, generator: dict):
from pathlib import Path
from mccode_antlr.reader import Reader
from mccode_antlr.reader import LocalRegistry
from mccode_antlr.translators.c import CTargetVisitor
args = mccode_script_parse(flavor)
config = dict(default_main=(not args.no_main) if args.no_main is not None else True,
enable_trace=args.trace if args.trace is not None else False,
portable=args.portable if args.portable is not None else False,
include_runtime=(not args.no_runtime) if args.no_runtime is not None else True,
embed_instrument_file=args.source if args.source is not None else False,
verbose=args.verbose if args.verbose is not None else False,
output=args.output_file if args.output_file is not None else args.filename.with_suffix('.c')
)
# McCode always requires access to a remote Pooch repository:
registries = [registry]
# A user can specify extra (local) directories to search for included files using -I or --search-dir
if args.search_dir is not None and len(args.search_dir):
registries.extend([LocalRegistry(d.stem, d) for d in args.search_dir])
# And McCode-3 users expect to always have access to files in the current working directory
registries.append(LocalRegistry('working_directory', f'{Path().resolve()}'))
# Construct the object which will read the instrument and component files, producing Python objects
reader = Reader(registries=registries)
# Read the provided .instr file, including all specified .instr and .comp files along the way
instrument = reader.get_instrument(args.filename)
# Construct the object which will translate the Python instrument to C
visitor = CTargetVisitor(instrument, generate=generator, config=config, verbose=config['verbose'])
# Go through the instrument, finish by writing the output file:
visitor.save(filename=config['output'])

And how its passed to the parser
def parse_mccode_instr(contents: str, registry: Registry, source: str = None) -> Instr:
from antlr4 import CommonTokenStream, InputStream
from mccode_antlr.grammar import McInstrParser, McInstrLexer
from mccode_antlr.instr import InstrVisitor
from mccode_antlr.reader import Reader
parser = McInstrParser(CommonTokenStream(McInstrLexer(InputStream(contents))))
registries = [registry]
reader = Reader(registries=registries)
visitor = InstrVisitor(reader, source or '<string>')
instr = visitor.visitProg(parser.prog())
instr.flags = tuple(reader.c_flags)
instr.registries = tuple(registries)
return instr

@g5t
Copy link
Collaborator

g5t commented Aug 28, 2024

Ah, so it was compiling the shared library! Sick that it didn't just error out :-)

So I am a bit confused about the relative paths you get from ncrystal-config, since I could have sworn they should be printed with absolute values.

It is absolute, I just shortened the paths for clarity.

And then I guess mccode-antlr and classic mcrun somehow orders the flags differently.

No, the order is the same. The difference is I pass the C source to the compiler via a pipe instead of writing a file (and then, currently, also write a file for debugging)

I agree we should try to make it more robust in any case... So -LCMD(ncrystal-config --show libdir) -l:CMD(ncrystal-config --show libname) would work

I just wasn't sure if, say clang, or MSVC, would handle this too

@tkittel
Copy link
Author

tkittel commented Aug 29, 2024

Clang is very similar to gcc, but MSVC probably in any case needs a completely different way of putting the flags.

I actually remembered, the solution is most likely just that I prepend -Wl, to the CMD(ncrystal-config --show libpath), since IIRC that actually means "for the linker, not the compiler". I prefer to avoid -L if possible, since then there is a higher chance of affecting other libraries being linked.

I was trying now with a local NCrystal_sample.comp in the rundir, but I keep getting (I renamed as _fixed in the filename and also inside it to avoid using the non-fixed one):

RuntimeError: NCrystal_sample_fixed not found in registry mcstas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants