Skip to content

Data submodule instead of CMake External Data#1000

Merged
bilke merged 15 commits intoufz:masterfrom
bilke:data-submodule
Feb 3, 2016
Merged

Data submodule instead of CMake External Data#1000
bilke merged 15 commits intoufz:masterfrom
bilke:data-submodule

Conversation

@bilke
Copy link
Copy Markdown
Member

@bilke bilke commented Jan 28, 2016

This PR ditches the CMake External Data stuff due to complicated handling and instead uses a git submodule in Tests/Data to provide test files. It will get checked out automatically when OGS_BUILD_TESTS=ON.

I also added an option (-o [dir]) to the cli to specify an output directory, see af0f0dc. Should the output directory be stored somewhere for later use? At the moment it is simply passed to solveProcesses().

The data submodule is a normal git repository (I also tried git-lfs but is slower when downloading / uploading and it is one dependency more because it has to be installed in addition to git). We just fill it with data and see if we hit some performance problems in the future...

Also unit tests can makes use of input files now. You can use data_path pointing [source-dir/Tests/Data]. For writing output files you can use data_binary_path pointing to [build-dir]/Tests/Data (see BuildInfo.h).

  • Update docs
  • Fix MPI ctests

@bilke
Copy link
Copy Markdown
Member Author

bilke commented Jan 28, 2016

🍹 Whohoo! I landed the 1000th pull request!! 🍹

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 28, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1115/

@wenqing
Copy link
Copy Markdown
Member

wenqing commented Jan 28, 2016

👏

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 28, 2016

Jenkins: OGS-6/Win-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Win-PRs/1012/

Comment thread BaseLib/FileTools.cpp
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If path is empty, the check will true, as far as i can see. That's good, but not obvious.
In general I'd prefer a method like join(path1, path2) to this one, because that would make sure that there are (back)slashes inserted appropriately whenever you use it.
If you don't agree, please at least document the behaviour for empty strings. In particular since that is the reason why OGS does not write to / when the new -o option is omitted. ✅

@chleh
Copy link
Copy Markdown
Collaborator

chleh commented Jan 28, 2016

😂

@chleh
Copy link
Copy Markdown
Collaborator

chleh commented Jan 28, 2016

Btw: There was that discussion about whether the CMake scripts should get the whole history of the test data repo by default. Do they, or do they restrict the history to, e.g., the most recent commit?

@bilke bilke force-pushed the data-submodule branch 5 times, most recently from 562279c to 6de7a57 Compare January 28, 2016 15:38
@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 28, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1125/

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 28, 2016

Jenkins: OGS-6/Mac-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Mac-PRs/1020/

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 28, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1126/

@bilke
Copy link
Copy Markdown
Member Author

bilke commented Jan 29, 2016

@chleh Unfortunately it is not possible to use shallow clones for submodules: if you just clone the most recent commit from a repos default branch and your submodule points to another commit in that repo things break...

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 29, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1130/

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 29, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1131/

@chleh
Copy link
Copy Markdown
Collaborator

chleh commented Jan 29, 2016

@bilke Have you considered this: http://stackoverflow.com/a/17692710
Maybe that solution is only applicable if done manually by the user. I don't know, I ain't a submodule guru.

@TomFischer
Copy link
Copy Markdown
Member

@bilke
Copy link
Copy Markdown
Member Author

bilke commented Jan 29, 2016

@chleh Yes I have seen this but it does not help.

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 29, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1132/

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jan 29, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1133/

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Feb 1, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1150/

@endJunction
Copy link
Copy Markdown
Member

Looks good and will definitely simplify updating of the tests. Hopefully it will not stress the github traffic too much.

⏩ after the petsc neumann test is fixed by Wenqing.

@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Feb 3, 2016

Jenkins: OGS-6/Linux-PRs failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs/1168/

@wenqing
Copy link
Copy Markdown
Member

wenqing commented Feb 3, 2016

@bilke I've copied the results to directory of /data/envinf/results_cube_1e3_neumann_pcs. You can copy the data from the directory and remove it late on.

@bilke
Copy link
Copy Markdown
Member Author

bilke commented Feb 3, 2016

Thanks! Please note that the correct name for reference files is [original_file_name]_expected.vtu.

bilke added a commit that referenced this pull request Feb 3, 2016
Data submodule instead of CMake External Data
@bilke bilke merged commit 63fd3a6 into ufz:master Feb 3, 2016
@bilke bilke deleted the data-submodule branch February 3, 2016 17:11
@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

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.

6 participants