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 issue #211 #214

Merged
merged 44 commits into from
Apr 20, 2023
Merged

Fix issue #211 #214

merged 44 commits into from
Apr 20, 2023

Conversation

brettviren
Copy link
Member

This needs to be applied on top of PR #199 (aka, it includes all those commits).

See last commit for details of the fix.

…test/bats

subrepo:
  subdir:   "test/bats"
  merged:   "a7106392"
upstream:
  origin:   "https://github.com/bats-core/bats-core.git"
  branch:   "master"
  commit:   "a7106392"
git-subrepo:
  version:  "0.4.5"
  origin:   "???"
  commit:   "???"
- Invert default of when to run tests.  Default is that they are OFF.

- Simplify test framework to remove "variant" tests from scope.

- Simplify by no longer bake "fast" vs "slow" or "unit" vs "integ" into test source file name.

- Bring `waf_unit_test` into `waft/wcb_unit_test` and move test argument handling there.

- Test related arguments will set during `configure` are saved to `build`.

- Add new feature to measure, store and report the run time for each test.

- Add new feature to exclude re-running slow tests based on a maximum duration limit.

- Update `framework.org` to match.
- Make the repo "live" only and entirely under build/tests and remove complexity of searching a "test data path".

- Add fillrepo and packrepo "wcb" commands to automate some management of test data repository files.

- Remove wct-bats.sh functions related to the old way of locting test data files, add/improve ones for the new way.

- Further purge the concept of "variant" tests and likewise downplay their opposites of "atomic" tests.

- Sync documentation and tests with these code/concept shifts.
Downgrade issue202 bats to a check as it is too long running
The "measure" is found by applying the connected components algorithm
to a special per-slice, per plane graph holding nodes representing
blobs and channels.  The nodes of this graph hold the vertex
descriptor of the original cluster graph node.

The bug was due to a channel being added to this special graph each
time it was seen across the blobs.  Thus, by construction, the
connected components (ie, measures) always had exactly one blob.

The fix was to only add any given channel exactly once.

The following test is extended to check this bug:

```
$ bats img/test/test-wct-uboone-img.bats
test-wct-uboone-img.bats
 ✓ create graph
 ✓ check log files
 ✓ inspect blobs
 ✓ dump blobs
 ✓ at least one multi-blob measure

5 tests, 0 failures
```
@HaiwangYu
Copy link
Member

@brettviren, I got some error trying to run the uboone-img test:
Screenshot 2023-04-18 at 2 58 27 PM

Which is related to no trace summary. Maybe you forgot to update the frame input here?

Screenshot 2023-04-18 at 2 58 56 PM

@HaiwangYu
Copy link
Member

The sim/sigproc tests seem OK. Just the sigproc results fluctuate again.

image

More notes: pr214.pdf

The Charge Solving results have much better consistency with WCP for multi-blob solving now:
Screenshot 2023-04-18 at 11 30 55 PM

@brettviren
Copy link
Member Author

Hi @HaiwangYu

Maybe you forgot to update the frame input

I think you are right. At least, the img/test/test-wct-uboone-img.bats test passes with my local "test data repo".

I've just refreshed the input and history tar files at https://www.phy.bnl.gov/~bviren/tmp/wcttest/data_repo/

Hopefully your running of the test will work now. To assure fresh download, you may need to:

rm -r build/tests/input*

Aside comment:

This out-of-sync problem would have been avoided if I remembered to do a distclean before a final test of the PR. But, fundamentally, the current form of the "test data repo" has no mechanism to tie its content to a particular git commit. For example, the new tar files I just uploaded likely break older versions of WCT tests in the git history. To provide a stronger connection between code and data versions I think the test data repo needs to move to something based on either git-annex or git-lfs. For the near ter, I think we continue to use tar files in a HTTP directory. Hopefully I can put something better before the next release, or perhaps the next-next one. I just made #215 to capture this.

@HaiwangYu
Copy link
Member

@brettviren, I still get an error which seems to be related to the numpy json diff test. But I think we could merge this PR first?
Screenshot 2023-04-19 at 4 16 29 PM

@brettviren
Copy link
Member Author

Yes, I think it is okay to merge. I'll just go ahead and do that. I'll look at checking the json vs numpy format test more.

@brettviren brettviren merged commit c7c832a into master Apr 20, 2023
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.

None yet

2 participants