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

JoyfulGen commented 2 weeks ago

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:

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 commented 2 weeks ago

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 commented 2 weeks ago

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

kyrieb-ekat commented 2 weeks ago

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 commented 2 weeks ago

DDMAL/Neon#1241

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

yinanazhou commented 2 weeks ago

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 commented 1 week ago

@yinanazhou, should this issue https://github.com/DDMAL/Neon/issues/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: https://github.com/DDMAL/Neon/issues/1243

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

yinanazhou commented 1 week ago

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

DDMAL / Rodan

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