Incorporating Julia code generation in the existing Python interface

[Ananya2003Gupta]

BEGINRELEASENOTES

• Added Julia code generation support in the existing Python interface.
• Implemented a new design structure for generated Julia code.
• Added default parameters in constructor definitions with support for Abstract types (for builtins).
• Created _sort_components_and_datatypes function to perform topological sort on components and datatypes.
• Created _has_static_arrays_import to check for the need of using Static Arrays in the generated julia code.
• Added --lang (-l) programming language argument to specify the programming language for code generation, current choices: cpp and julia, default: cpp.
• Added --upstream-edm code generation support for julia.
• Added tests in the unit test suite, covering the Julia code generation of the example data models.
• Added documentation for julia code generation.
• Added ENABLE_JULIA toggle option. By default it is OFF.

ENDRELEASENOTES

[graeme-a-stewart]

Hi - looks pretty good, a few comments embedded. Are the tests as extensive as the tests for the C++ rendering of the data model? It's not too exhaustive.

Also, we need to rethink the "forward declaration" solution we use too.

[graeme-a-stewart]

Maybe long long should be Int128? I am not sure though, as this would be a slow datatype in practice.

[tmadlener]

long long is 64 bits in practice at the moment (as far as the table on the integers shows). We could put in a header that encodes these "assumptions" into the c++ code, that essentially consists of static_asserts of the form

static_assert(sizeof(long long int) == 8, "long long int is not 64 bits wide");

[graeme-a-stewart]

BTW, you also failed some pylint tests that should be fixed

[Ananya2003Gupta]

I have corrected all the changes that were asked.

[jmcarcell]

Is this optional? I would say even if Julia is installed there will be cases when we'll want podio not to have the Julia interface by default. Of course if Julia is not installed then it should work as it works now, tests included

[Ananya2003Gupta]

Is this optional? I would say even if Julia is installed there will be cases when we'll want podio not to have the Julia interface by default. Of course if Julia is not installed then it should work as it works now, tests included

Yes, the code generation will work regardless of whether julia is present in the system or not, as julia code for a particular datamodel is generated using jinja2 templates. And yes the tests for julia will only run if the users have julia in their system.

[Ananya2003Gupta]

Is this optional? I would say even if Julia is installed there will be cases when we'll want podio not to have the Julia interface by default. Of course if Julia is not installed then it should work as it works now, tests included

Also, that's why we have introduced a new parameter --lang (-l). By default the language is cpp, but if the users want to generate julia code, they can by specifying -l=julia or --lang=julia.

[graeme-a-stewart]

Hi - I think this isn't the last word on the EDM in Julia, but it works and allows us to develop from here without very painful rebases, so I am happy to merge this now.

[hegner]

@Ananya2003Gupta @graeme-a-stewart - I'd agree once the conflicts are resolved ;-)

[Ananya2003Gupta]

@Ananya2003Gupta @graeme-a-stewart - I'd agree once the conflicts are resolved ;-)

Yes, it's done : )

[graeme-a-stewart]

Hi @Ananya2003Gupta

I got a test failure when I was running in your branch:

ERROR: LoadError: ArgumentError: Package StaticArrays not found in current path.
- Run `import Pkg; Pkg.add("StaticArrays")` to install the StaticArrays package.

Can you fix that one? You shouldn't make assumptions about which packages are installed in the test environment.

[tmadlener]

I think this would just need to be put into the README as a dependency and also how to install it, right? CI seems to be configured to do that and the tests are running and succeeding.

[Ananya2003Gupta]

Hi @Ananya2003Gupta

I got a test failure when I was running in your branch:

ERROR: LoadError: ArgumentError: Package StaticArrays not found in current path.
- Run `import Pkg; Pkg.add("StaticArrays")` to install the StaticArrays package.

Can you fix that one? You shouldn't make assumptions about which packages are installed in the test environment.

Actually you would have to install StaticArrays package in your system, then only it would work.
This can be done: https://discourse.julialang.org/t/install-a-packages-from-a-list/30920/6

[Ananya2003Gupta]

I think this would just need to be put into the README as a dependency and also how to install it, right? CI seems to be configured to do that and the tests are running and succeeding.

Yes. The users need to have StaticArrays package installed in their system. Otherwise it will throw this error and ask them to install it. We can make it a dependency as suggested here: https://discourse.julialang.org/t/install-a-packages-from-a-list/30920/6

[graeme-a-stewart]

Actually you would have to install StaticArrays package in your system, then only it would work. This can be done: https://discourse.julialang.org/t/install-a-packages-from-a-list/30920/6

But you should not fail the test, we are testing the code, not the user's environment. Probably the best way is to have a Project.toml specifically for the test.

For the user, document that StaticArrays is required, but the user should not have to install it in their default Julia environment (which is really bad practice).

[Ananya2003Gupta]

Actually you would have to install StaticArrays package in your system, then only it would work. This can be done: https://discourse.julialang.org/t/install-a-packages-from-a-list/30920/6

But you should not fail the test, we are testing the code, not the user's environment. Probably the best way is to have a Project.toml specifically for the test.

For the user, document that StaticArrays is required, but the user should not have to install it in their default Julia environment (which is really bad practice).

Okay

[Ananya2003Gupta]

I have added a try-catch block in unittest.jl to check if StaticArrays is available, if not, install it locally.

[Ananya2003Gupta]

@hegner @tmadlener can you please review the PR? If there are anymore changes required I would do them.

[tmadlener]

I can't comment too much on the generated Julia code. @graeme-a-stewart and @hegner are better equipped to comment on that. There are a few minor things (see also the inline comments)

• I would merge params.jinja2 and abstracttypes.jinja2 into one julia_helpers.jinja2 file and include that where necessary in the main jinja2 template files
• The unittest.jl file should be in tests/unittests, rather than in tests
• I would like to have an option ENABLE_JULIA in the top level CMakeLists.txt to toggle Julia support manually, rather than relying on finding the Julia executable alone.
• Can you bring back the BEGINRELEASENOTES and ENDRELEASENOTES in the top level comment of the PR? Those are used as delimiter for our tagging script.

More generally, I am not sure whether mixing in the Julia code generator into the existing c++ code generator is the best idea, as that is already complicated enough on its own and sprinkling a few if lang == "xxx" in there doesn't help in that regard. Having said that, I think we can also fix this after this PR and ideally split them into to separate generator classes that each handle exactly one language.

[tmadlener]

The basic idea for splitting classes would be to have a sort of base class that does the common processing and them two dedicated classes for cpp and julia respectively that deal with the specifics. However, as I said, I would first get this PR merged as it is with the things mixed and then we can separate things later, when there are no multiple other PRs touching code generation in flight, so that we don't have to keep all of them synchronized.

[Ananya2003Gupta]

The basic idea for splitting classes would be to have a sort of base class that does the common processing and them two dedicated classes for cpp and julia respectively that deal with the specifics. However, as I said, I would first get this PR merged as it is with the things mixed and then we can separate things later, when there are no multiple other PRs touching code generation in flight, so that we don't have to keep all of them synchronized.

Got it.
Also, I am not sure why the key4hep / build-and-test (sw-nightlies.hsf.org/key4hep, ON) (pull_request) is failing.

 [374/381] Linking CXX shared library tests/schema_evolution/libTestDataModel_v3SioBlocks.so
  [375/381] Generating TestDataModel_v3Dict.cxx, TestDataModel_v3DictDict.rootmap
  Warning: Unused class rule: ExampleWithUserInitv2
  Warning: Unused class rule: ex42::ExampleWithNamespacev2
  Warning: Unused class rule: ExampleWithArrayv2
  [376/381] Building CXX object tests/schema_evolution/root_io/CMakeFiles/write_old_data_root.dir/write_old_data_root.cpp.o
  [377/381] Linking CXX executable tests/schema_evolution/root_io/write_old_data_root
  [378/381] Building CXX object tests/schema_evolution/root_io/CMakeFiles/read_new_data_root.dir/read_new_data_root.cpp.o
  [379/381] Building CXX object tests/schema_evolution/CMakeFiles/TestDataModel_v3Dict.dir/TestDataModel_v3Dict.cxx.o
  [380/381] Linking CXX shared library tests/schema_evolution/libTestDataModel_v3Dict.so
  [381/381] Linking CXX executable tests/schema_evolution/root_io/read_new_data_root
  ninja: build stopped: cannot make progress due to previous errors.
  Error: Process completed with exit code 1.

[tmadlener]

That looks like a temporary issue unrelated to your changes here: https://github.com/AIDASoft/podio/actions/runs/7003341864/job/19048875848?pr=473#step:4:986

We will simply use the next run if there are more changes, or rerun this later and it should pass.

[Ananya2003Gupta]

I would like to have an option ENABLE_JULIA in the top level CMakeLists.txt to toggle Julia support manually, rather than relying on finding the Julia executable alone.

For this, should I keep an ENABLE_JULIA toggle like ENABLE_SIO ? If julia executable is found, enable the toggle to on and then run the tests written for julia in tests/CMakeLists.txt?

[Ananya2003Gupta]

That looks like a temporary issue unrelated to your changes here: https://github.com/AIDASoft/podio/actions/runs/7003341864/job/19048875848?pr=473#step:4:986

We will simply use the next run if there are more changes, or rerun this later and it should pass.

Okay. Yes I need to make one more change for ENABLE_JULIA. I may do the rebase as well. And force push the changes.

[tmadlener]

For this, should I keep an ENABLE_JULIA toggle like ENABLE_SIO ? If julia executable is found, enable the toggle to on and then run the tests written for julia in tests/CMakeLists.txt?

Yes, I think doing the same thing as ENABLE_SIO is a good idea, i.e. if the users wants Julia, but doesn't have Julia we can simply fail at cmake stage.

[Ananya2003Gupta]

For this, should I keep an ENABLE_JULIA toggle like ENABLE_SIO ? If julia executable is found, enable the toggle to on and then run the tests written for julia in tests/CMakeLists.txt?

Yes, I think doing the same thing as ENABLE_SIO is a good idea, i.e. if the users wants Julia, but doesn't have Julia we can simply fail at cmake stage.

Okay. So, the datamodels for julia would only be generated when ENABLE_JULIA is ON and so the julia tests will run?

[tmadlener]

Yes, exactly.

[Ananya2003Gupta]

I have added the ENABLE_JULIA toggle button.
We can re run the two failed tests.

[Ananya2003Gupta]

@tmadlener I have made all the asked changes.

[tmadlener]

Very nice. Thank you. I have just a few final minor comments. I can also just do them via the github web interface.

I will let @hegner or @graeme-a-stewart press the button.