tecan / ttc21incrementalLabWorkflows

This repository contains the code for the Incremental Laboratory Workflow case at the TTC 2021
MIT License
0 stars 3 forks source link

Review of the Fulib Solution #8

Open arturboronat opened 3 years ago

arturboronat commented 3 years ago

The Fulib solution features:

The solution is presented in plain Java and, despite Java's verbosity, reads well and clear using an imperative approach. So this is a good point in favour of Fulib. The performance smashes the performance of the reference solution and of all other solutions that work with separate object models.

Metamodel changes were worth it in order to explore possible optimizations in performance and the case study has succeeded in demonstrating this.

However, I could not find the Java classes implementing the solution at: https://github.com/sekassel/ttc2021fuliblabworkflow/tree/master/src/main/java/fulib/labworkflow. Changing the metamodels also means that the baseline for comparing the different solutions no longer applies.

Overall, an interesting achievement in performance results! Could similar performance results be achieved without modifying the given object models?

fjouault commented 3 years ago

The Fulib solution is imperative, relatively compact, and readable. It seems to be quite scalable.

We noticed the following issues in the result models. Because the solution cannot be run by the benchmark script, we cannot confirm at this time whether they would be detected by the NMF-based validation program or not.

The solution runner (FulibSolution.java) seems to incorrectly apply some changes:

Some issues may be explained by the fact that some changes were performed on the source and target metamodels, as reported in the paper. The authors wrote that they did not realize that it would break things when they applied these changes.

Remark: this review is based on the code from branch “main” of the Fulib solution repository, because branch “master” seemed obsolete.

This review was written in collaboration with @TheoLeCalvar.