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

[Staging] Glyphs are placed an octave too high on the staff #1216

Open
JoyfulGen opened this issue Oct 10, 2024 · 7 comments
Open

[Staging] Glyphs are placed an octave too high on the staff #1216

JoyfulGen opened this issue Oct 10, 2024 · 7 comments

Comments

@JoyfulGen
Copy link
Contributor

Here's a demo where you can see that the glyphs are appearing an octave above where they should be. It's a bit messy, so I've put boxes around places where the issue is clear:

Glyphs octave too high

I suspect this is related to a fix to the Heuristic_Pitch_Finding job to make the default octave of clefs C4 instead of C3, because it looks like the pitches of the neumes have been fixed, but not the clefs. When staves don't have clefs, as in the example above, Neon assigns them a default C clef on the third line of the staff (from the bottom). That clef is still C3, but the neumes are now pitched an octave up, so they appear an octave too high. @lucasmarchd01, is this possible?

@lucasmarchd01
Copy link
Contributor

Right, thanks for pointing this out! I think what's happening here is that previously in Neon, we would account for the incorrect clef location by adjusting the neume pitches an octave up. Now that the clef octave is correct, the pitches now appear an octave too high. Are you able to confirm that the MEI file output is correct?

Then, I think this becomes a Neon issue for the correct display. @yinanazhou might be able to help with this?

@yinanazhou
Copy link
Member

Yesss, this is related to DDMAL/Neon#1241. I will push the changes this weekend. And Rodan needs to be updated to point to the that commit.

@kyrieb-ekat
Copy link

kyrieb-ekat commented Oct 11, 2024

Note on this; this might also affect any jobs using the NIC and Pitch Finding in the NIC though anything is, of course, possible! I was running a OMR workflow on staging and got the following error:

RuntimeError: Image view dimensions out of range for data nrows 47 offset_y 511 data nrows 46 data offset_y 512 ncols 56 offset_x 256 data ncols 215 data offset_x 256

Which might be from the pitches being off the page, though I'm not sure. I couldn't reproduce on production, as production is failing when the NIC is run ~75% of the time (bug hunt for this in progress, updates to come...).

Here's the full traceback:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/celery/app/trace.py", line 412, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/celery/app/trace.py", line 704, in __protected_call__
    return self.run(*args, **kwargs)
  File "/code/Rodan/rodan/jobs/base.py", line 791, in run
    retval = self.run_my_task(inputs, settings, arg_outputs)
  File "/code/Rodan/rodan/jobs/heuristic_pitch_finding/base.py", line 183, in run_my_task
    pitches = pf.get_pitches(glyphs, staves)
  File "/code/Rodan/rodan/jobs/heuristic_pitch_finding/PitchFinding.py", line 45, in get_pitches
    self._find_pitches(self.glyphs)
  File "/code/Rodan/rodan/jobs/heuristic_pitch_finding/PitchFinding.py", line 94, in _find_pitches
    center_of_mass = self._x_projection_vector(g)
  File "/code/Rodan/rodan/jobs/heuristic_pitch_finding/PitchFinding.py", line 143, in _x_projection_vector
    temp_glyph,y_add = self.get_subimage(g,this_punctum_size_cols,this_punctum_size_rows)
  File "/code/Rodan/rodan/jobs/heuristic_pitch_finding/PitchFinding.py", line 167, in get_subimage
    ((glyph.offset_x + 1.0 * extend_by_cols - 1), (glyph.offset_y + glyph.nrows -1)))
  File "/usr/local/lib/python3.7/site-packages/gamera/core.py", line 503, in subimage
    return SubImage(self, *args, **kwargs)
RuntimeError: Image view dimensions out of range for data
	nrows 47
	offset_y 511
	data nrows 46
	data offset_y 512
	ncols 56
	offset_x 256
	data ncols 215
	data offset_x 256

@homework36
Copy link
Contributor

DDMAL/Neon#1241

Do you know where is that line in Rodan repo to update Neon?

@yinanazhou
Copy link
Member

DDMAL/Neon#1241

Do you know where is that line in Rodan repo to update Neon?

I just checked the last time we updated Rodan/Neon (#865). Rodan now uses Neon:develop. In this case, @JoyfulGen could you confirm that neon staging is in good shape for me to push all the changes to neon production? If so, I would merge the changes.

@JoyfulGen
Copy link
Contributor Author

JoyfulGen commented Oct 14, 2024

@yinanazhou, should this issue DDMAL/Neon#1227 be fixed on staging? Or have the changes not been pushed yet? The issue is still happening, but I don't know if that's expected or not...

I also found this one: DDMAL/Neon#1243

But that's it! After that it's all good to go :)

@yinanazhou
Copy link
Member

@JoyfulGen This is expected. The changes haven't been merged into verovio. I will have a look at the new one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants