Avoid that the OMR processes finishes prematurely #53
Conversation
|
The ruff error in the CI pipeline should disappear as soon as #52 is merged |
* Fixed 'TypeError: Cannot convert 4.999899999999999e-07 to EagerTensor of dtype int64' in training, fixes BreezeWhite#39 https://stackoverflow.com/questions/76511182/tensorflow-custom-learning-rate-scheduler-gives-unexpected-eagertensor-type-erro * --format was deprecated in ruff and replaced wtih --output-format
There was a problem hiding this comment.
Hi, sorry for the late reply. I've been on a vacation recently.
I have admit that I've forgot lots of the implementation details, and thus may not correctly justify whether the modifications are proper or not. Rather, I choose to trust the refactoring makes sense. Just a few comments need to be confirmed.
Thanks again for making this PR.
oemer/rhythm_extraction.py
Outdated
| try: | ||
| cur_scan_line = note_id_map[int(start_y):int(bbox[3]), int(right_bound)] | ||
| ids = set(np.unique(cur_scan_line)) | ||
| if -1 in ids: | ||
| ids.remove(-1) | ||
| if len(ids) > 0: | ||
| break | ||
| right_bound += 1 | ||
| if right_bound >= bbox[2] + unit_size: | ||
| break | ||
| except IndexError as e: | ||
| print(e) |
There was a problem hiding this comment.
Maybe just this scope is enough
try:
cur_scan_line = note_id_map[int(start_y):int(bbox[3]), int(right_bound)]
except IndexError as e:
...| raise E.StafflineCountInconsistent( | ||
| f"Some of the stafflines contains less or more than 5 lines: {line_num}") | ||
| print(f"Some of the stafflines contains less or more than 5 lines: {line_num}") | ||
| continue |
There was a problem hiding this comment.
I'm not sure if ignoring these exceptions is good or not, since the later flow relies heavily on strong stafflines assumptions.
There was a problem hiding this comment.
In the cases I saw, with the change you get e.g. 4 out of 5 staffs. The results then weren't great. I think what the change mainly allows is to understand better which staff (or part of the image) caused the problems. Like you can see in the teaser, what staffs have been detected and which ones haven't. With the exception, it's much harder to understand that as you also don't get a teaser image.
But that said, I'm also fine to revert this change.
There was a problem hiding this comment.
Maybe you can do some tests on those cases with and without these modifications to see if removing the exception really effects the later process?
| raise E.StafflineNotAligned( | ||
| f"Centers of staff parts at the same row not aligned (Th: {horizontal_diff_th}): {norm(centers)}") | ||
| print(f"Centers of staff parts at the same row not aligned (Th: {horizontal_diff_th}): {norm(centers)}") | ||
| continue |
| if not np.all(norm(unit_size) < unit_size_diff_th): | ||
| raise E.StafflineUnitSizeInconsistent( | ||
| f"Unit sizes not consistent (th: {unit_size_diff_th}): {norm(unit_size)}") | ||
| if not np.all(norm(unit_size) < unit_size_diff_th): | ||
| print(f"Unit sizes not consistent (th: {unit_size_diff_th}): {norm(unit_size)}") | ||
| continue | ||
| valid_staffs.append(staffs) |
oemer/symbol_extraction.py
Outdated
| raise E.SfnNoteTrackMismatch(f"Track of sfn and note not mismatch: {ss}\n{note}") | ||
| if ss.group != note.group: | ||
| raise E.SfnNoteGroupMismatch(f"Group of sfn and note not mismatch: {ss}\n{note}") | ||
| notes[ss.note_id].sfn = ss.label | ||
| ss.is_key = False | ||
| print(f"Track of sfn and note not mismatch: {ss}\n{note}") | ||
| notes[ss.note_id].invalid = True | ||
| elif ss.group != note.group: | ||
| print(f"Group of sfn and note not mismatch: {ss}\n{note}") | ||
| notes[ss.note_id].invalid = True | ||
| else: | ||
| notes[ss.note_id].sfn = ss.label | ||
| ss.is_key = False |
There was a problem hiding this comment.
Same as above.
And also sorry for the typo 😂 Should be "Group of sfn and note mismatch", the "not" is redundant.
…are different for the same source code level on different test runs due to update of the dependencies
|
Happy new year! This is an example for the staffline exceptions:
On the main branch is should give you this error: "oemer.exceptions.StafflineNotAligned: Centers of staff parts at the same row not aligned (Th: 0.1): [0.14311853 0.14315237 0.14318621 0.14334265 0.14335062 0.1434011 The issue here is the background which is dark and noisy. On this branch you instead get a result which looks like this in MuseScore:
While there are plenty of issues in the result, I still think it's useable. The teaser looks quite good:
Let me know what you think and if you would prefer to keep the exceptions or keep the PR as it is. |
|
Well...though the result still has lots of room to be improved, but still surprised that |
|
Thanks. I was mostly using oemer with pictures taken from phone cameras. It really did an impressive job if I compared the results to other tools. |
|
Thanks for the appreciation. So is it good to merge this PR? Anything need to modify? |
|
Yes, I believe the PR can be merged. |
* Fixed typos in comments * IndexError while scanning for a dot should not abort the whole process * Bound check while getting the note label * Added check if label is in the note_type_map * Filter staffs instead of aborting with an exception * Bound check during symbol extraction * Marking notes as invalid instead of aborting with an exception * Bound check * Fixed type error * Fixed TypeError at start of unet or segnet training (BreezeWhite#52) * Fixed 'TypeError: Cannot convert 4.999899999999999e-07 to EagerTensor of dtype int64' in training, fixes BreezeWhite#39 https://stackoverflow.com/questions/76511182/tensorflow-custom-learning-rate-scheduler-gives-unexpected-eagertensor-type-erro * --format was deprecated in ruff and replaced wtih --output-format * HoughLinesP can return None if no lines are found * Fixed error which happens if no rest bboxes were found * Limited try/except block * Fixed typo * Use fixed versions for the linter dependencies to avoid that results are different for the same source code level on different test runs due to update of the dependencies * Fixed type errors which came up with the recent version of cv2 * Going back to the newest version of ruff and mypy as the type errors were introduced by cv2
…ingle entry point to all training steps (#54) * Fixed 'TypeError: Cannot convert 4.999899999999999e-07 to EagerTensor of dtype int64' in training, fixes #39 https://stackoverflow.com/questions/76511182/tensorflow-custom-learning-rate-scheduler-gives-unexpected-eagertensor-type-erro * --format was deprecated in ruff and replaced wtih --output-format * Added a single entry point to train all models * Added convenience wrapper for oemer * Tried to figure out the definitions for the dense dataset and to document them in code There is likely an official definition somewhere but I just couldn't find it. So I looked at example and tried to reconstruct the mapping. Unknown basically means that I just couldn't see the symbol on the picture. * Decreased queue sizes as otherwise the training process crashed with an out of memory exception after it used up about 30GB of memory * Added model outputs to git ignore * Added checks for dataset folders * Using default training params * Added workarounds for removal of np.float * Using dataset definitions * Added type annotations * Added a train_all_rests even if the resulting model is right now not used in oemer * segnet and unet should now pick the correct model * Changed label definitions from what appears to be used in oemer right now * With this commit the resulting arch.json matches the one inside of oemer/ceckpoints/seg_net/arch.json * Avoid that the OMR processes finishes prematurely (#53) * Fixed typos in comments * IndexError while scanning for a dot should not abort the whole process * Bound check while getting the note label * Added check if label is in the note_type_map * Filter staffs instead of aborting with an exception * Bound check during symbol extraction * Marking notes as invalid instead of aborting with an exception * Bound check * Fixed type error * Fixed TypeError at start of unet or segnet training (#52) * Fixed 'TypeError: Cannot convert 4.999899999999999e-07 to EagerTensor of dtype int64' in training, fixes #39 https://stackoverflow.com/questions/76511182/tensorflow-custom-learning-rate-scheduler-gives-unexpected-eagertensor-type-erro * --format was deprecated in ruff and replaced wtih --output-format * HoughLinesP can return None if no lines are found * Fixed error which happens if no rest bboxes were found * Limited try/except block * Fixed typo * Use fixed versions for the linter dependencies to avoid that results are different for the same source code level on different test runs due to update of the dependencies * Fixed type errors which came up with the recent version of cv2 * Going back to the newest version of ruff and mypy as the type errors were introduced by cv2 * Fix install from github command in README --------- Co-authored-by: Yoyo <miyashita2010@tuta.io>
Hi,
This is just a collection of small fixes for various exceptions and errors I experienced while running oemer through various images. It's mainly bound checks. The changes also try to filter out invalid results instead of raising errors in the assumptions that it's better to try to extract as much as possible.