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

Rework result processing #219

Merged
merged 37 commits into from
Aug 7, 2023
Merged

Rework result processing #219

merged 37 commits into from
Aug 7, 2023

Conversation

nck-mlcnv
Copy link
Contributor

@nck-mlcnv nck-mlcnv commented Jul 6, 2023

This PR mainy aims to renew the result processing as it was quite messy. Changes:

  • Properties class isn't used anymore, which was quite unwieldy
  • most metrics don't write RDF models anymore and only calculate single number values
  • added usage of static RDF objects
  • penalized metrics are now their own metrics, penalty needs to be given in the configuration now
  • "new" metric "Aggregated Executions Statistics"
  • the Stresstest class is now managing the result processing itself
  • changed repository structure

@nck-mlcnv nck-mlcnv changed the title Feature/rework result processing Rework result processing Jul 6, 2023
This was linked to issues Jul 6, 2023
@nck-mlcnv nck-mlcnv linked an issue Jul 7, 2023 that may be closed by this pull request
- metrics should also now be created before the storages
@nck-mlcnv nck-mlcnv linked an issue Jul 11, 2023 that may be closed by this pull request
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it looks good. I see a lot of things that will change with the next open PR #218 is finished. So I tend towards merging this one fast into a new branch major-rewrite/develop that we branch from develop and continue then with #218.

@nck-mlcnv nck-mlcnv requested a review from bigerl July 21, 2023 14:25
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. There is nothing in the comments that should still be resolved in this PR. So, we are good to go to merge this.

@@ -184,19 +175,9 @@ protected HttpContext getAuthContext(String endpoint) {
}

public synchronized void addResults(QueryExecutionStats results) {
// TODO: check if statement for bugs, if the if line exists in the UpdateWorker, the UpdateWorker fails its tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can be ignored for now because the UpdateWorker will be rewritten anyway.


Model m = ModelFactory.createDefaultModel();

m.add(experimentRes, RDF.type, IONT.experiment);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use method cascading with add

this.suiteFolder = folder.resolve(IguanaConfig.getSuiteID());
}

public static Arguments createTestData1() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data generation of this test is highly fragile in a sense that small changes to the rdf serialization render the generated data incorrec.t. It could be more robust if it relied on StresstestResultProcessor directly to generate a jena model.
This applies in a similar fashion to the other tests in this package as well. This doesn't need to be patched but can be moved to an issue.

@bigerl bigerl merged commit f44b101 into develop Aug 7, 2023
2 checks passed
@bigerl bigerl deleted the feature/rework-result-processing branch August 7, 2023 10:53
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

Successfully merging this pull request may close these issues.

Update documentation Repository structure review Results in CSV format Refactor RDF usage
2 participants