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

Installation of additional tools with ASTE #200

Open
davidscn opened this issue Sep 4, 2024 · 5 comments
Open

Installation of additional tools with ASTE #200

davidscn opened this issue Sep 4, 2024 · 5 comments

Comments

@davidscn
Copy link
Member

davidscn commented Sep 4, 2024

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

MAPPING_TESTER="${TEST_LOCATION}"/../../tools/mapping-tester/

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

@fsimonis
Copy link
Member

fsimonis commented Sep 4, 2024

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).

@davidscn
Copy link
Member Author

davidscn commented Sep 4, 2024

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,

@uekerman
Copy link
Member

uekerman commented Sep 4, 2024

I am also in favor of installation and fully documenting this additional UI. In the end, it is also documentation for us.

@davidscn
Copy link
Member Author

davidscn commented Sep 5, 2024

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

@fsimonis
Copy link
Member

fsimonis commented Sep 5, 2024

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

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

3 participants