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

Fix composable blueprints #348

Open
kaspermarstal opened this issue Feb 25, 2019 · 8 comments
Open

Fix composable blueprints #348

kaspermarstal opened this issue Feb 25, 2019 · 8 comments
Assignees

Comments

@kaspermarstal
Copy link
Member

The Composable blueprints in ContinuousRegistration/Submissions/ComposableBlueprints are not working. This is due to two issues:

  • SuperElastix cannot find the included path. We should add logic to SuperElastix that resolves the path the included blueprint. The included path should be the path relative to the blueprint that includes it. SuperElastix should use provide the full path to the included blueprint to the boost loader.
  • SuperElastix has gained additional components in recent weeks. This means the criteria in the composable blueprints would previously be enough to specify a unique component, but now it is not. We should more criteria to these blueprints. The "NameOfClass" criterion might be enough to solve this issue.
@N-Dekker
Copy link
Member

@kaspermarstal

The included path should be the path relative to the blueprint that includes it.

So this one does not need to be supported, right?

"./ContinuousRegistration/Submissions/ComposableBlueprints/Composables/ITKv4_base.json"

In ContinuousRegistration\Submissions\ComposableBlueprints\ITKv4_SyN_ANTsNC.json

@N-Dekker
Copy link
Member

The existing approach appears to assume that ContinuousRegistration is a subdirectory of the current working directory. Is that a problem?

N-Dekker added a commit that referenced this issue Feb 26, 2019
Relative paths in the "Include" section of a Blueprint file are now resolved relative to the file path of the composing Blueprint.

Requested by @kaspermarstal issue #348 "Fix composable blueprints".
@N-Dekker
Copy link
Member

N-Dekker commented Mar 1, 2019

Does the merge of pull request #350 solve this issue, or is it still necessary to implement "the second bullet" of this issue?

@kaspermarstal
Copy link
Member Author

Yes, that is still necessary. Two composable blueprints at the leaderboard still have N/A results :(

@N-Dekker
Copy link
Member

N-Dekker commented Mar 4, 2019

@kaspermarstal

Two composable blueprints at the leaderboard still have N/A results

So these two json files still need to be adjusted, right?

  1. https://github.com/SuperElastix/SuperElastix/blob/develop/ContinuousRegistration/Submissions/ComposableBlueprints/AffineTransform.json
  2. https://github.com/SuperElastix/SuperElastix/blob/develop/ContinuousRegistration/Submissions/ComposableBlueprints/BSplineTransformPOPI.json

Where can I find the error messages on the server? Or do you know how to fix them already?

@FBerendsen
Copy link
Member

I have a case where the relative paths are not working correctly (i.e. in the bash scripts generated by CRC to run locally); the call looks like this:
> [relative_path_a]/SuperElastix --conf [relative_path_b]/blueprint_with_relative_includes.json [...]
It is working if I cd to [relative_path_a] and correct [relative_path_b] accordingly.

Originally posted by @FBerendsen in #350 (comment)

@FBerendsen
Copy link
Member

FBerendsen commented Mar 14, 2019

Correction:
It is working if I cd to [relative_path_b] and correct [relative_path_a] accordingly.

@N-Dekker
Copy link
Member

@FBerendsen With 7508a1f the paths of the included json files are either absolute, or relative to the path of the including json file. The current working directory (cd) should not matter, when finding the included json files. Are you sure it does?

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