Open davidscn opened 1 month ago
I think installing them would be the most logical from the perspective of a user. We could add an option to disable the installation of these extra tools, similar to PRECICE_BUILD_TOOLS
in the precice project.
We would also probably need to install the configuration template somewhere predictable.
We also need to come up with a comprehensive naming scheme for these tools, or even merge them into a single script with subcommands. Something like precice-aste-mapping-tester-generate
looks a bit lengthy.
Installation aside, using these scripts without installing/while developing is also tedious as seen in the run scripts of the tests.
This is a similar situation to the precice-profiling
script in the precice repo.
It is installed, but not present in the binary directory as it isn't compiled.
Copying it to the binary directory requires rerunning cmake to update the copy when modifying the script, alternatively one could use file(CREATE_LINK dst name COPY_ON_ERROR SYMBOLIC)
.
We could add an option to disable the installation of these extra tools, similar to PRECICE_BUILD_TOOLS in the precice project.
Yes, agree, I think that's a great idea. We can then also execute the corresponding test conditionally depending on the user config.
or even merge them into a single script with subcommands
While this could be an option, doing this sounds like a rather large refactoring. We could also use a naming like precice-aste-tools-generate
or similar? Still lengthy, I agree..
Copying it to the binary directory requires rerunning cmake to update the copy when modifying the script, alternatively one could use file(CREATE_LINK dst name COPY_ON_ERROR SYMBOLIC)
Modifying source code requires rerunning the compiler as well. There is also value in having these files in the build tree, as it allows users to simply 'export' the build tree and with this having a complete installation. Otherwise, users have to know which scripts are required, where to find them and export them individually,
I am also in favor of installation and fully documenting this additional UI. In the end, it is also documentation for us.
Ok, how do you want to look this like from the user perspective?
At the moment we have in aste/tools/mapping-tester
:
.
├── config-template.xml
├── gatherstats.py
├── generate.py
├── plotconv.py
└── preparemeshes.py
Brainstorming a bit:
Given that this tool is currently not installed nor documented, we could also clarify the name before continuing. mapping tester
is anyway a strange name as it doesn't test mappings. Its more of an analysis / parameter study tool.
Therefore, we could rename it to study
, making the prefix precice-aste-study-
or even precice-study-
.
In aste/tools/study/
we have:
config-template.xml
which we need no matter what and could be installed in prefix/share/aste/study/config-template.xml
. Then in the tool, default to ./config-template.xml
and if missing search XDG_DATA_DIRS
(which normally contains the install prefix/share) for the template.generate.py
-> precice-study-generate
preparemeshes.py
-> precice-study-prepare
or precice-study-pre
(process)gatherstats.py
-> precice-study-gather
or precice-study-post
(process)plotconv.py
-> precice-study-plot convergence
then we have one script where we can implement common plots.Halfway off-topic: together with #195, this could form a good foundation for performance regression testing
Our examples illustrate an example usage of the mapping tester. The main motivation for adding it there was back then to have it tested in our CI. However, the example runs successfully, because we export the location of the (tools) scripts as part of the run script
https://github.com/precice/aste/blob/302cf1e7ca5a6c73c71d68cdd89c885fa500696b/examples/mapping_tester/run.sh#L9
If users want to make use of these tools, they need to do export their location manually or make them discoverable, because they are not installed as part of ASTE. It would be easy to add them as install targets in CMake, which would make them part of the UI (also considering their naming), however. Opinions on what would be an expected behavior here @uekerman @fsimonis ? Also loosely related to #195, as the tester itself is not part of the documentation at the moment.
Triggered by #199