Skip to content

Conversation

@supersergiy
Copy link
Collaborator

Automatic testing for corgie.

Overall, bloats the repo by 200MB.

@supersergiy supersergiy requested a review from nkemnitz May 14, 2021 23:49
@nkemnitz
Copy link
Contributor

nkemnitz commented May 15, 2021

About the size issue, two things I can think of:

  • If / where possible, generate test data procedurally. E.g., testing translation adjustment or vector voting won't require real vector fields, just some affine matrices converted to vector fields (which can happen as part of the test) - or even simpler, constant fields... depends on the test, I guess.
  • Place the ground truth data in another repo, e.g corgie-testdata and include it here only as submodule. That way you don't force people to download the entire data, especially if we make revisions to those test files.

@supersergiy
Copy link
Collaborator Author

The data can actually be generated procedurally by running generate_all_gt.sh on the master branch.

Can we merge it like this for now and then revisit it when we need to add more gt? 200MB is not too bad

@nkemnitz
Copy link
Contributor

Oh, but if that is the case, then I would definitely not add the ground truth to the Github repo. What's the issue with calling generate_all_gt.sh locally for everybody who wants to contribute/test?
If you replace or remove files later, git pull will still download the entire 200 MiB, even if the data is long gone from the repo.

@supersergiy
Copy link
Collaborator Author

The only issue it that it takes around 20 mins to generate, and I was worried that people will mess generation up, aka generate the gt on the wrong branch, forget to update their gt with changes to the repo etc. But you're right, we probably don't want this 200MB to hang around.

How about I'll ditch the data for now in favor of download_original.sh and generate_all_gt.sh scripts, and then if we have any issues then I'll make a subrepo with the data?

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.

2 participants