Export to GitHub

lilypond - issue #2745

clef change confuses manual key signature


Posted on Aug 14, 2012 by Quick Lion

From <URL:http://lists.gnu.org/archive/html/lilypond-user/2012-08/msg00265.html&gt;:

In the following: \version "2.14.2" \score { \relative c' { \time 2/4 \set Staff.keySignature = #`(((0 . 4) . ,SHARP) ((0 . 3) . ,SHARP)) \clef treble c8 a c d %%% Commenting out the following line solves the problem %%% \clef bass e fis d c } \layout {} }

The clef change causes lilypond to error and not produce output. This also errors in 2.15., while 2.12 does not error.

Comment #1

Posted on Aug 14, 2012 by Quick Lion

Issue 2745: clef change confuses manual key signature

http://codereview.appspot.com/6450128

Comment #2

Posted on Aug 14, 2012 by Quick Kangaroo

Patchy the autobot says: passes tests.

Comment #3

Posted on Aug 14, 2012 by Quick Lion

I have looked through the other accidental functions, and all this is a mess. From a mail I wrote on this topic http://lists.gnu.org/archive/html/lilypond-user/2012-08/msg00279.html>:

Ok, consider me annoyed now. Yes, we have some snippets documenting this sort of thing, but what is it even supposed to mean?

The actual accidental code knows two kinds of accidental entries: one without octave entry for the key signature, and one with octave entry and bar/measure position for signifying a locally changed key signature by a particular accidental in the music with given note and octave and time.

The actual code does not try making sense of a key signature entry with octave (and consequently without bar/measure position). And what is a key signature with octave location actually supposed to mean? Do we need an accidental for a note in key signature but one octave higher, or not?

So I fail to make any sense of your example. If I had to guess, I'd say the octave specifications are there for overriding the default octaves chosen by the key signature engraver, but without being fixed to a certain octave concerning their effect on the music.

However, with that interpretation, a clef change like you propose above leads to accidentals displayed up in the sky in ledger line domain. What's the key engraver to do in this case? Transpose the whole octave-enriched key signature down by entire octaves (thus keeping the arrangement of the scale) until it starts making sense again? Leave it in the sky with ledger lines? Without?

What is your expectation? For what kind of music and situation is this useful?

I have the suspicion that this is a documentation problem more than anything else: I don't really see the sense in a key signature with octave specifications.

Comment #4

Posted on Aug 14, 2012 by Quick Lion

Here is an example typeset with a somewhat fixed version: \relative c' { \accidentalStyle teaching \set Staff.keySignature = #`(((0 . 6) . ,FLAT) ((0 . 5) . ,FLAT) ((0 . 3) . ,SHARP)) fis f' fis, fis \clef bass \break fis aes4 bes, c }

The teaching accidental style has the consequence that accidentals not immediately following a note with the same accidental get a cautionary accidental. So what do we get here? See attachment in next comment (gimp tends to kill my X server, so I have to put this comment out first).

Comment #5

Posted on Aug 14, 2012 by Quick Lion

See comment #4. What sense is that supposed to make?

Attachments

Comment #6

Posted on Aug 14, 2012 by Quick Lion

Removing "Regression" tag. While it may be true that this did not cause a Scheme error in 2.12, I am unconvinced that it caused useful behavior. I am afraid that solving this requires defining useful behavior, documenting it, and then implementing it. It may be feasible to submit a patch curing the Scheme error, but that does not mean that useful behavior will fall out.

Comment #7

Posted on Aug 15, 2012 by Quick Lion

(No comment was entered for this change.)

Comment #8

Posted on Aug 16, 2012 by Quick Lion

Issue 2745: clef change confuses manual key signature

http://codereview.appspot.com/6450128

Comment #9

Posted on Aug 16, 2012 by Quick Kangaroo

Patchy the autobot says: passes tests.

Comment #10

Posted on Aug 17, 2012 by Quick Lion

Issue 2750 has been merged into this issue.

Comment #11

Posted on Aug 17, 2012 by Swift Camel

Build results are available at

http://grenouille.lilynet.net/patches-tests/2745/ 21:26:38 Success: git apply --index /home/jmandereau/lilypond-extra/patches/issue6450128_4001.diff 21:26:38 Success: sudo -u lilybuild ./autogen.sh --noconfigure 21:27:05 Success: sudo -u lilybuild ../configure --disable-optimising 21:27:11 Success: sudo -u lilybuild nice make clean 21:43:48 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 22:05:15 Success: sudo -u lilybuild nice make check -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

Comment #12

Posted on Aug 17, 2012 by Happy Kangaroo

Sorry for the half-baked upload in previous comment, I intended to have test-patches upload regtests comparison, not just the logs, but I botched something in Patchy code. I've tried to fix, and I launched it again... this is still in a bloody alpha phase, but at least I believe no to give patched Lily source tree and build system access to Git push, email sending and Google grenouille account access.

Comment #13

Posted on Aug 18, 2012 by Swift Camel

Build results are available at

None

23:25:01 Success: git apply --index /home/jmandereau/lilypond-extra/patches/issue6450128_4001.diff 23:25:01 Success: sudo -u lilybuild ./autogen.sh --noconfigure 23:25:28 Success: sudo -u lilybuild ../configure --disable-optimising 23:25:34 Success: sudo -u lilybuild nice make clean 23:42:07 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 00:03:02 Success: sudo -u lilybuild nice make check -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

Comment #14

Posted on Aug 18, 2012 by Quick Lion

http://grenouille.lilynet.net/patches-tests/2745/test-results/> points to serious problems (undead objects).

They can't be really caused by this patch. So it would appear that there is something else wrong in the test setup.

Comment #15

Posted on Aug 18, 2012 by Swift Camel

Build results are available at

http://grenouille.lilynet.net/patches-tests/2745/

01:05:18 (UTC) Begin LilyPond compile, previous commit at 229e78d781e24469d65e47244fb6eb161be84621 01:05:28 Success: sudo -u lilybuild ./autogen.sh --noconfigure 01:05:56 Success: sudo -u lilybuild ../configure --disable-optimising 01:06:06 Success: sudo -u lilybuild nice make clean 01:22:38 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 01:42:08 Success: sudo -u lilybuild nice make test-baseline -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

01:42:08 Success: git apply --index /home/jmandereau/lilypond-extra/patches/issue6450128_4001.diff 01:42:09 Success: sudo -u lilybuild ./autogen.sh --noconfigure 01:42:36 Success: sudo -u lilybuild ../configure --disable-optimising 01:42:41 Success: sudo -u lilybuild nice make clean 01:59:15 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 02:20:11 Success: sudo -u lilybuild nice make check -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

Comment #16

Posted on Aug 18, 2012 by Happy Kangaroo

The logs from make show that I build with "-O2 -finline-functions", could this be the cause of warnings "Parsed object should be dead" added or removed in log diffs?

Comment #17

Posted on Aug 18, 2012 by Quick Lion

"Parsed object should be dead" is a message concerning object types that should have been garbage-collected at the end of a LilyPond file. Garbage collection is finicky and may well differ under different optimization options.

Now if an object is not dead at the end of one file, it is unlikely to be dead at the end of the next file, so the collating manner in which files get compiled in a random order makes the diff spectacularly useless for figuring out just what test leaves undead objects.

Maybe I need to enter them in a weak hash table in order to ignore them once they have been announced.

Comment #18

Posted on Aug 19, 2012 by Quick Lion

The last few comments concern the Grenouille experiments and are not relevant to that topic. The discussion has more or less cleared up that there is need for and previous scores using octave-specific key signatures in the context of scordatura (detuned string instruments written down in fingered rather than sounding pitch).

With regard to non-octave-specific signatures with non-standard octave positioning, it would seem to make sense to adapt the signature printer rather than misuse an octave-containing signature data structure.

So the documentation and the code are lacking functionality and information. While the existing functionality is limited in scope and ill-documented regarding what it can and cannot sensibly do, this particular fix removes a bug in the limited functionality available.

The original bug report might contain ill-advised usage of key signatures due to the documentation and functionality problem. This fix will probably not help the original poster all that much to achieve his actual full goal.

It still addresses several bugs and makes that part of the functionality that is available work under more conditions.

So it should go on countdown.

Comment #19

Posted on Aug 20, 2012 by Happy Camel

(No comment was entered for this change.)

Comment #20

Posted on Aug 22, 2012 by Happy Camel

Counted down to 20120821, please push.

Comment #21

Posted on Aug 22, 2012 by Quick Lion

Pushed to staging as commit 11c215ed719156dd51359fb2d7e47803a2efb5ec Author: David Kastrup Date: Tue Aug 14 06:15:33 2012 +0200

Issue 2745: clef change confuses manual key signature

The original design of the accidental code apparently had two modes
for entries: a (notename . alter) pair for entries belonging to the
key signature, and a ((octave . notename) . (alter barnum . measurepos))
construct for actually occuring accidentals affecting the key signature
locally.  The documentation, however, also specifies an
((octave . notename) . alter) form for key signatures.  The code in general
has not been quite prepared for this.  This changes at least the accessor
functions for either car or cdr of a notename entry to only look at
their part of the signature entry.

Comment #22

Posted on Aug 23, 2012 by Quick Ox

We had two regression tests trying to cover scordatura, where these manual key signatures with specified octaves were useful. One needed to redefine the key signatures before any clef changes to get useful output, but it used to work without crashing.

So I'll flag this as Regression, just in case anyone is willing to spend the effort for back-ports.

For the case of key signatures with the normal meaning (affecting all octaves) but custom printing, we have enhancement issue 2768.

Comment #23

Posted on Aug 23, 2012 by Quick Lion

Issue 2745: more fun with accidental rules.

There was still code mixing up the somewhat orthogonal concept of single-octave accidentals with non-keysignature accidentals.

Also a bug in teaching accidental rules which never saw accidentals in the key signature.

http://codereview.appspot.com/6476051

Comment #24

Posted on Aug 23, 2012 by Quick Lion

(No comment was entered for this change.)

Comment #25

Posted on Aug 23, 2012 by Quick Lion

Patchy the autobot says: accidentalStyle teaching misses cautionary accidentals in accidental-contemporary.ly

Comment #26

Posted on Aug 29, 2012 by Swift Camel

Build results are available at

http://grenouille.lilynet.net/patches-tests/2745/test-results

13:56:31 (UTC) Begin LilyPond compile, previous commit at 1cdcb2f73f35834fef5cbbc2a6647a11ae468f85 13:56:38 From git.savannah.gnu.org:/srv/git/lilypond 1cdcb2f..66a7c3e master -> master 13:56:49 Merged master, now at: 66a7c3e925cbc1a34eaad04f80d4bc42ad9834ac 13:56:51 Success: sudo -u lilybuild ./autogen.sh --noconfigure 13:57:28 Success: sudo -u lilybuild /home/lilybuild/master/configure --disable-optimising 13:57:38 Success: sudo -u lilybuild nice make clean 14:13:02 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 14:34:46 Success: sudo -u lilybuild nice make test-baseline -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

19:09:44 Issue 2745: clef change confuses manual key signature 19:09:44 Issue 2745: Testing patch issue6476051_1_diff 19:09:44 Success: sudo -u lilybuild git apply --index /home/jmandereau/lily-test-patches/issue6476051_1.diff 19:09:46 Success: sudo -u lilybuild ./autogen.sh --noconfigure 19:10:12 Success: sudo -u lilybuild /home/lilybuild/master/configure --disable-optimising 19:10:22 Success: sudo -u lilybuild nice make clean 19:25:41 Success: sudo -u lilybuild nice make -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1 19:48:21 Success: sudo -u lilybuild nice make check -j2 CPU_COUNT=2 ANTI_ALIAS_FACTOR=1

Comment #27

Posted on Sep 2, 2012 by Quick Kangaroo

This is labelled 'needs_work' but also 'fixed' which is it?

Comment #28

Posted on Sep 3, 2012 by Quick Lion

Looks like an incomplete fix (addressing the original report but not being entirely consistently) has been pushed and is contained in 2.17.0. A followup fix has been posted, but the review has been clobbered by discussions about Grenouille and the need to do separate work. It also appears that the latest patch trying to address additional inconsistencies causes problems described in comment #25, so it is indeed "Needs_work", though the current state is not likely a regression any more due to the first, already pushed change. As a further complication, a complementing change has already been pushed as issue 2768.

Comment #29

Posted on Sep 9, 2012 by Quick Lion

Issue 2745: more fun with accidental rules.

There was still code mixing up the somewhat orthogonal concept of single-octave accidentals with non-keysignature accidentals.

Also a bug in teaching accidental rules which never saw accidentals in the key signature.

http://codereview.appspot.com/6476051

Comment #30

Posted on Sep 9, 2012 by Quick Lion

Patchy the autobot says: passes tests. The same random two undead objects less than mainline as the unrelated parallel test, so definitely just a fluke.

Comment #31

Posted on Sep 10, 2012 by Happy Camel

(No comment was entered for this change.)

Comment #32

Posted on Sep 11, 2012 by Happy Lion

I think this was a finger slipping, since it's on the email countdown.

Comment #33

Posted on Sep 12, 2012 by Happy Camel

Counted down to 20120911, please push.

Yes, two fingers slipped last Sunday!

Comment #34

Posted on Sep 12, 2012 by Quick Lion

Pushed to staging as commit bbca17061fbc7a25ca4050e6d031b3e9562b658e Author: David Kastrup Date: Thu Aug 23 15:39:04 2012 +0200

Issue 2745: more fun with accidental rules.

There was still code mixing up the somewhat orthogonal concept of
single-octave accidentals with non-keysignature accidentals.

Comment #35

Posted on Sep 24, 2012 by Happy Camel

(No comment was entered for this change.)

Comment #36

Posted on Oct 19, 2012 by Quick Lion

Pushed to stable/2.16 as commit 4261503b7bbe41042d65062542ca96bacae89005 Author: David Kastrup Date: Thu Aug 23 15:39:04 2012 +0200

Issue 2745: more fun with accidental rules.

There was still code mixing up the somewhat orthogonal concept of
single-octave accidentals with non-keysignature accidentals.

Comment #37

Posted on Jan 29, 2015 by Massive Lion

Just for the record, in case this issue ever gets resurrected: there is a case for octave-specific key signatures in early renaissance music. Specifically, where the music entails a mixture of "soft" and "hard" hexachords (whence, incidentally, the names b-mol and b-dur derive, as do our modern signs for flat and natural).
[Apel: The notation of polyphonic music, 900-1600. Available at https://archive.org/stream/notationofpolyph00apel#page/460/mode/2up where the discussion is indexed under "partial signature")

As a concrete example, I've transcribed parts of the Carver Choirbook (Scotland, early sixteenth century), using % altus \clef "treble" \set Staff.keySignature = #(((-1 . 6) . ,NATURAL) (( 0 . 6) . ,FLAT)) % ... % bassus \clef "bass" \set Staff.keySignature = #(((-2 . 6) . ,FLAT) ((-1 . 6) . ,NATURAL)) and lilypond (2.19.5) seems to behave perfectly. (Haven't needed to transpose though). -- Graham King

Status: Verified

Labels:
Type-Defect Fixed_2_17_3 Fixed_2_16_1