POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

Including/Syncing small xml examples with the Orchestrator repo #195

Closed m8pple closed 2 years ago

m8pple commented 3 years ago

It would be useful to have an explicit link between:

While the documentation talks about plate_3x3.xml, it is not clear whether this is intended to be the same file as the version in Orchestrator_examples/plate_heat/pregen/plate_3x3.xml. The biggest issue is between Orchestrator and orchestrator-examples - as a user I want to be pretty certain that I'm running the right XML variant on the right orchestrator.

Two ways of doing this could be:

  1. Have a sub-directory in Orchestrator that contains xml files which are part of the test-suite for that version of the orchestrator. These could be pretty simple, and ideally would be the ones mentioned in the documentation.

  2. Include Orchestrator_examples and orchestrator-documentation as sub-modules, so that they can be versioned alongside the orchestrator.

It doesn't have to be the entire set of xml files you are using for unit/integration testing in CI - just some small subset would be helpful, so that a user can sanity check that their installation works, and that it works with the installed version of Tinsel.

mvousden commented 3 years ago

While the documentation talks about plate_3x3.xml

As far as I can tell, the documentation (1.0.0-alpha) doesn't ever refer to plate_3x3.xml. The user guide uses ring_test.xml, and points out that it's in the examples repository.

As a general rule, I'm against the idea of including documentation and examples in the same repository as source. I'm also against the idea of using submodules for this purpose, as the orchestrator source doesn't depend on the documentation or the examples. Traditionally, this problem is addressed via a "meta repository", which joins all repositories of one project together, but do we really need that for something with only three repos associated with it?

I also think it's clear that 1.0.0-alpha examples work with the 1.0.0-alpha source and the 1.0.0-alpha documentation. We do a good job of having a unified branching structure between the three repositories, and all PRs and issues link across them as necessary. Any failure of that link is an issue that either needs to be addressed, or is "under address" via development work or a PR.

However, I welcome discussion! @heliosfa ?

m8pple commented 3 years ago

As noted on #264 if you don't want unit testing in the Orchestrator repo, feel free to strip it out of the PR.

Personally, I have found it quite hard working across three repos that are loosely synchronised, particularly as the main branch moves backwards and forwards between 1.0.0-alpha and development. The relationship between Orchestrator-examples and the Orchestrator is particularly unclear:

I understand the idealism of keeping code and testing separate, but in practise I personally find it makes life harder. I'm also not saying I do a perfect job of keeping everything up to do date in my repos (I definitely don't), but there is at least the mild comfort that if you clone the repo and do make test then you get a general sense of how well things work in that revision.

In graph_schema running make test does about 60 different integration tests involving compiling tools, generating XML, and simulating it in multiple ways. Half the time it is a bit broken, but at least you know it is broken in that revision. Part of the reason we were able to adapt graph_schema for V3 so quickly is that there are a ton of automated unit and integration tests. Similarly, it took about 3 days after the V4 XML standardisation meeting (2 years ago) for Jonny to add V4 parsing and generation to graph_schema, partially due to the ability to run the tests over and over.

But again: if you don't want unit tests in the source repo, fair enough. Strip them out and just take the parsing fixes, and I'll work out whether it's worth maintaining the Orchestrator tests for my own purposes.

mvousden commented 3 years ago

As noted on #264 if you don't want unit testing in the Orchestrator repo, feel free to strip it out of the PR.

We should absolutely have unit tests in the repository. We already do for the hardware description file reader, and for the placement (in a rudimentary way). It would be good to integrate the tests you have created into the Orchestrator repository.

Personally, I have found it quite hard working across three repos that are loosely synchronised,

Pretty much every large actively-developed software project I've interacted with using or worked on splits work into different repositories (using a meta-repo), for separation of concerns. If we want to go with the monolith approach and put everything in the same repository, then simulators should come in. Papers should come in. Where does one draw the line?

particularly as the main branch moves backwards and forwards between 1.0.0-alpha and development.

I apologise for this confusion. This change that happened later than we intended. Part of the reason we wanted to make a release was to attract people to use it... which has been a sticking point for a while.

The relationship between Orchestrator-examples and the Orchestrator is particularly unclear:

  • is it a test-bench, or just examples?

There are only examples in the Orchestrator-examples repository. There are some files labelled test_ which have been used to demonstrate functionality in the past (ala system tests), but no unit tests live in that repository unless they pertain to application generation.

  • Which commit of the orchestrator is it relative to?

Orchestrator's development maps to Orchestrator-examples' development. The same holds for the documentation repository.

  • Should everything in Orchestrator-examples work?

Yes, outside of feature branches of course.

  • If I clone the Orchestrator what command do I run to try to work out if it "works"?

Compile the tests:

cd Build/gcc
make tests

Run the tests:

./run-tests.sh  # From the root directory of the Orchestrator
  • If I change the Orchestrator which repo do I go to to find out if I have caused a regression?

If you've changed the Orchestrator, the Orchestrator is where you go to find out if you've caused a regression (I think I'm misunderstanding your question).

  • There is XML in the repo that is clearly v2 or v3 XML. Is it still an example?

I can't find any there from a cursory glance. Can you please point them out? The micromagnetics submodule is undergoing a rewrite at present (and doesn't have any pregenerated examples for this reason).

I understand the idealism of keeping code and testing separate, but in practise I personally find it makes life harder. I'm also not saying I do a perfect job of keeping everything up to do date in my repos (I definitely don't), but there is at least the mild comfort that if you clone the repo and do make test then you get a general sense of how well things work in that revision.

In graph_schema running make test does about 60 different integration tests involving compiling tools, generating XML, and simulating it in multiple ways. Half the time it is a bit broken, but at least you know it is broken in that revision. Part of the reason we were able to adapt graph_schema for V3 so quickly is that there are a ton of automated unit and integration tests. Similarly, it took about 3 days after the V4 XML standardisation meeting (2 years ago) for Jonny to add V4 parsing and generation to graph_schema, partially due to the ability to run the tests over and over.

But again: if you don't want unit tests in the source repo, fair enough. Strip them out and just take the parsing fixes, and I'll work out whether it's worth maintaining the Orchestrator tests for my own purposes.

I completely agree with the above. Has anyone said it would be a bad idea to keep unit tests in the same repository as the source? Would be best if they were hooked into the existing testing infrastructure though.

mvousden commented 3 years ago

If you prefer, we could create an orchestrator-meta repo, which contains the other repositories using git subtree?

m8pple commented 2 years ago

See the propose change to readme which discusses tests (not sure if it works TBH, but no obvious errors)

mvousden commented 2 years ago

What proposed changes?

m8pple commented 2 years ago

Ah, looks like the "convert branch to pull request" timed out on conference wifi.

See pull request #319