dwagelaar / ttc2023-kmehr2fhir

The TTC 2023 KMEHR to FHIR case
MIT License
0 stars 5 forks source link

Open review for ATOL solution #9

Open agarciadom opened 1 year ago

agarciadom commented 1 year ago

I was able to build and run the solution using the provided code, after adding atol to the list of tools in the config/config.json file. I did have to make sure I defined the GITHUB_ACTOR and GITUB_TOKEN environment variables to those of a personal token with the read:packages permission before running the script, but this doesn't seem to have been documented anywhere. Could the solution authors add a README.md to their solution that clarifies this?

Once I got the results, I copied over the src/main/viz folder in my Epsilon solution and quickly touched up the diagrams.R script to produce figures on memory/runtime. These pretty much reproduce the results on the paper, drawing up the same comparisons (ATOL being faster but significantly more hungry on memory).

I'll continue adding comments on the quality of the artifact and its paper below.

Memory, initialisation

memory-Initialization

Memory, load

memory-Load

Memory, run

memory-Run

Runtime, initialization

runtime-Initialization

Runtime, load

runtime-Load

Runtime, run

runtime-Run

agarciadom commented 1 year ago

I notice that the .generated.atl file seems to have its rules in a different order, so I couldn't easily do a side-by-side diff to compare the two. One thing that I've liked to see in the paper is whether there is any type of tool support to let an ATL user know whether their .atl file has anything that wouldn't work in ATOL (e.g. some kind of IDE warning/error).

I found the frequent -- @type X comments a bit distracting. Are these needed, or is it just for helping debug the higher-order transformation?

There is also a change adding .value132 to the AdministrativeGender rule, which I didn't quite understand. Is .value132 part of the original metamodel, or is it something new produced by ATOL?

image

The RESOLVE helper seems rather hard to understand. Is your end goal to have users write regular ATL scripts and have the HOT "compile down" to the ATOL-compatible subset of ATL, or would you expect users to write directly ATOL-compatible scripts, including such resolvers?

Overall, this seems very impressive technically, but it'd be nice to know what is your end goal in ATOL regarding the intended user experience.

agarciadom commented 1 year ago

Regarding the paper, I found it generally to be easy to follow for someone already familiar with ATL. There are some minor typos and tweaks to be done for the post-proceedings version, some of which I will list below.

Minor tweaks: