Issue 1278: Arrow notation for quarter-tones.

Issue 1278: Arrow notation for quarter-tones.

We change the internal pitch representation so as to account for two
levels of alteration. Basically, instead of representing alterations
by a rational number, we represent them by a pair of integers.

The main changes are the addition of an Alteration struct, and
modifications in the Pitch and Scale classes.

The Pitch class was reimplemented. Its interface was mostly preserved. 
No pitch normalisation policy is implemented, though.
(see lily/include/pitch.hh and lily/pitch.cc)

Class Scale was completely rewritten.
(see lily/include/scale.hh and lily/scale.hh)

Finally, new note names are defined, and the arrow style accidentals
are made available via the new arrow-alteration-glyph-name-alist.
(see scm/lily-library.scm, scm/define-note-names.scm and
scm/output-lib.scm)

Other changes are basically simple side-effects of these.

[Felipe, 2010-12-30 07:26:08]

This patch is first and foremost a change in the internal Pitch representation.
I then suggest that we first discuss that, having it as a reference.

[Graham Percival (old account), 2011-01-28 14:17:01]

Hi Felipe,

I'm very sorry about the delay, but at least I'm looking at it now, and I'll
take care of badgering people to review it once it's ready.

Unfortunately due to the delay, we have a few extra regtests, which fail when
this patch is applied.  One is tablature-string-tunings.ly -- I think that the
old (ly:make-pitch ...) just needs to be updated to use the NATURAL thing that
you've used elsewhere.

Could you try rebasing this patch from current git master, check for anything
that's changed from 4 weeks ago, and then upload a new draft?

I promise that we won't ignore the next draft.  I'm very sorry about this
problem.
- Graham

[felipeg.assis_gmail.com, 2011-01-29 10:38:20]

Right,

I will have a look at that.

Thanks!

Felipe

On 28 January 2011 12:17,  <percival.music.ca@gmail.com> wrote:
> Hi Felipe,
>
> I'm very sorry about the delay, but at least I'm looking at it now, and
> I'll take care of badgering people to review it once it's ready.
>
> Unfortunately due to the delay, we have a few extra regtests, which fail
> when this patch is applied.  One is tablature-string-tunings.ly -- I
> think that the old (ly:make-pitch ...) just needs to be updated to use
> the NATURAL thing that you've used elsewhere.
>
> Could you try rebasing this patch from current git master, check for
> anything that's changed from 4 weeks ago, and then upload a new draft?
>
> I promise that we won't ignore the next draft.  I'm very sorry about
> this problem.
> - Graham
>
> http://codereview.appspot.com/3789044/
>

[Graham Percival (old account), 2011-02-10 18:13:40]

Sorry, I still can't apply this cleanly to git master.  Are you sure you did a
rebase?  (I admit that I'm not totally certain about how to do this, but I've
seen people in lilypond-devel talking about it -- maybe look through the mailing
list archives for tips?)

At the moment, I see failures in
  lily/key-engraver.cc
and warnings in
  lily/parser.yy
  ly/engraver-init.ly
  scm/translation-functions.scm

[felipeg.assis_gmail.com, 2011-02-10 19:35:25]

Hi,

In fact, yesterday's "make key cancellations independent of extraNatural"
introduced a simple conflict with my patch, which I have just resolved.

Other files were automatically merged.

After some testing, I will upload a new patch-set.

Regards,
Felipe

On 10 February 2011 16:13,  <percival.music.ca@gmail.com> wrote:
> Sorry, I still can't apply this cleanly to git master.  Are you sure you
> did a rebase?  (I admit that I'm not totally certain about how to do
> this, but I've seen people in lilypond-devel talking about it -- maybe
> look through the mailing list archives for tips?)
>
> At the moment, I see failures in
>  lily/key-engraver.cc
> and warnings in
>  lily/parser.yy
>  ly/engraver-init.ly
>  scm/translation-functions.scm
>
>
> http://codereview.appspot.com/3789044/
>

[Graham Percival (old account), 2011-02-10 22:32:07]

Thanks!  Regtests look fine, and there's nothing obviously wrong with the code.

However, none of the scm files managed to get uploaded.  I remember hearing
about problems about this, and the CG now contains:
-----
Note: Some installations of git-cl fail when uploading a patch set that includes
a .scm file. When this happens, it can generally be fixed by editing the file
‘/etc/mime.types’. Add a line to this file containing text/x-script.scheme scm. 
-----
on page: 
http://lilypond.org/doc/v2.13/Documentation/contributor/commits-and-patches#u...

Could you try going that, then uploading again?

[felipeg.assis_gmail.com, 2011-02-13 14:32:48]

Hi,

A new patch set was uploaded, now including the scm files.

On 10 February 2011 20:32,  <percival.music.ca@gmail.com> wrote:
> Thanks!  Regtests look fine, and there's nothing obviously wrong with
> the code.
>
> However, none of the scm files managed to get uploaded.  I remember
> hearing about problems about this, and the CG now contains:
> -----
> Note: Some installations of git-cl fail when uploading a patch set that
> includes a .scm file. When this happens, it can generally be fixed by
> editing the file ‘/etc/mime.types’. Add a line to this file containing
> text/x-script.scheme scm.
> -----
> on page:
>
http://lilypond.org/doc/v2.13/Documentation/contributor/commits-and-patches#u...
>
> Could you try going that, then uploading again?
>
> http://codereview.appspot.com/3789044/
>

[Graham Percival (old account), 2011-02-14 22:27:51]

I think it's ready for review now.

http://codereview.appspot.com/3789044/diff/29001/ly/makam.ly
File ly/makam.ly (right):

http://codereview.appspot.com/3789044/diff/29001/ly/makam.ly#newcode9
ly/makam.ly:9: #(ly:set-default-scale (ly:make-scale '#((0 . 0) (0 . 0) (0 . 0)
(-9 . 0) (-9 . 0) (-9 . 0) (-9 . 0) (-18 . 0)) 1 1/18 0))
Could we get some linebreaks in here?

http://codereview.appspot.com/3789044/diff/29001/scm/lily.scm
File scm/lily.scm (right):

http://codereview.appspot.com/3789044/diff/29001/scm/lily.scm#newcode387
scm/lily.scm:387: (ly:set-default-scale (ly:make-scale #((0 . 0) (0 . 0) (0 . 0)
(-1 . 0) (-1 . 0) (-1 . 0) (-1 . 0) (-2 . 0)) 1 1/2 1/4))
linebreaks here could be helpful.

[MikeSol, 2011-02-17 19:03:22]

LGTM.

I admittedly don't know this part of the source well, so I'll leave comments
about that to other programmers.

My only concern is about the scope of this patch.  It seems like this patch is
doing two things: implementing a new type of quarter tone notation &
implementing alterations as a replacement for rationals in certain scenarios. 
From the headache that sometimes comes with having to go upstream with the code
after something accidentally breaks, it seems that patches should generally be
confined to one and only one functionality that is modified.

Would it be possible, then, to separate out you work with accidentals from your
work with alterations so that, if something like git bisect needs to be done,
any eventual problem can be localized.

[felipeg.assis_gmail.com, 2011-02-17 19:27:46]

Hi,

In fact, this patch is divided in a few commits in my local branch.
The ones which change the pitch representation are most of the
diff. The rest are trivial additions that enable the new notation
to be used.

If this patch is approved, there will be no problem in separating
the work in two or three commits.

If you want me to upload just part of the patch, I will do so,
but that wouldn't simplify the diffs much.


On 17 February 2011 17:03,  <mtsolo@gmail.com> wrote:
> LGTM.
>
> I admittedly don't know this part of the source well, so I'll leave
> comments about that to other programmers.
>
> My only concern is about the scope of this patch.  It seems like this
> patch is doing two things: implementing a new type of quarter tone
> notation & implementing alterations as a replacement for rationals in
> certain scenarios.  From the headache that sometimes comes with having
> to go upstream with the code after something accidentally breaks, it
> seems that patches should generally be confined to one and only one
> functionality that is modified.
>
> Would it be possible, then, to separate out you work with accidentals
> from your work with alterations so that, if something like git bisect
> needs to be done, any eventual problem can be localized.
>
> http://codereview.appspot.com/3789044/
>

[Carl, 2011-02-17 20:01:25]

This work looks good to me, but I'm not an expert in this area.

I have one question, I think.  Right now, the alteration consists of two
integers, which have implied denominators of 1/2 and 1/4, if I understand
correctly.

Would it be more general to have the alteration consist of two rationals?  Or
could we have some way of defining a base-alteration pair that would be
rationals?  And then the implied denominators of 1/2 and 1/4 would no longer be
implicit, but would be explicit in either the alteration or the base-alteration?

I think (but am not sure) that such an implementation would meet the needs that
Hans Aberg has defined for appropriate microtonal support.

Of course, this entire suggestion could be garbage, because I am not an expert
on this topic, as I mentioned above.

Thanks,

Carl

http://codereview.appspot.com/3789044/diff/29001/lily/accidental-placement.cc
File lily/accidental-placement.cc (right):

http://codereview.appspot.com/3789044/diff/29001/lily/accidental-placement.cc...
lily/accidental-placement.cc:239: Alteration last_alteration (0);
Why is this last_alteration (0), instead of last_alteration () or
last_alteration (NATURAL)?

http://codereview.appspot.com/3789044/diff/29001/lily/include/pitch.hh
File lily/include/pitch.hh (right):

http://codereview.appspot.com/3789044/diff/29001/lily/include/pitch.hh#newcode60
lily/include/pitch.hh:60: Pitch (int octave, int notename, int alt1, int alt2);
I'm somewhat hesitant about this code that limits an alteration to two integers.
 Are we sure that this is general enough given the fact that we're moving to an
Alteration datatype?

http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc
File lily/scale.cc (right):

http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc#newcode36
lily/scale.cc:36: " arguments are rational numbers giving the weigth in"
spelling: weigth -> weight

http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc#newcode94
lily/scale.cc:94: " scale.  The number of pitches in this scale minus one"
"The number of scale steps in an octave is the number of pitches in the global
scale minus one."

http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py
File python/musicexp.py (right):

http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py#newcode308
python/musicexp.py:308: return '(ly:make-pitch %d %d \'(%d . 0))' %
(self.octave,
Why is the 0 for the second alteration hard-coded?

http://codereview.appspot.com/3789044/diff/29001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

http://codereview.appspot.com/3789044/diff/29001/scm/define-markup-commands.s...
scm/define-markup-commands.scm:2418: (interpret-markup layout props (markup
#:musicglyph (assoc-get '(2 . 0) standard-alteration-glyph-name-alist ""))))
Can this use SHARP, instead of (2 . 0)?

[Graham Percival, 2011-02-17 20:19:59]

On 2/17/11, Carl.D.Sorensen@gmail.com <Carl.D.Sorensen@gmail.com> wrote:
> I have one question, I think.  Right now, the alteration consists of two
> integers, which have implied denominators of 1/2 and 1/4, if I
> understand correctly.
>
> Would it be more general to have the alteration consist of two
> rationals?  Or could we have some way of defining a base-alteration pair
> that would be rationals?  And then the implied denominators of 1/2 and
> 1/4 would no longer be implicit, but would be explicit in either the
> alteration or the base-alteration?
>
> I think (but am not sure) that such an implementation would meet the
> needs that Hans Aberg has defined for appropriate microtonal support.

Very good point!  I've cc'd Hans and Graham Breed to inquire about this.

Current patch is here:
http://codereview.appspot.com/3789044/

Cheers,
- Graham

[felipeg.assis_gmail.com, 2011-02-17 20:34:35]

Hi Carl,

The denominators are not implicit.
They are set in the file scm/lily.scm via the call to ly:set-default-scale.

Also see lily/scale.cc, lily/include/scale.hh and lily/pitch.cc:89
for how this is implemented.

ly/makam.ly gives an example of how to modify the denominators.

If you need further clarification, just ask me.

I will reply to the other comments, and possibly upload a new revision
by the weekend.

[Felipe, 2011-02-17 20:39:13]

http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py
File python/musicexp.py (right):

http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py#newcode308
python/musicexp.py:308: return '(ly:make-pitch %d %d \'(%d . 0))' %
(self.octave,
On 2011/02/17 20:01:25, Carl wrote:
> Why is the 0 for the second alteration hard-coded?

IIUC, this python code only deals with semitones.
By the way, the corresponding line in the current version
is a bug (I sent a mail to the frogs last week about that).

[c_sorensen, 2011-02-18 04:35:00]

On 2/17/11 1:34 PM, "Felipe Gonçalves Assis" <felipeg.assis@gmail.com>
wrote:

> Hi Carl,
> 
> The denominators are not implicit.
> They are set in the file scm/lily.scm via the call to ly:set-default-scale.
> 
> Also see lily/scale.cc, lily/include/scale.hh and lily/pitch.cc:89
> for how this is implemented.

Ahh, OK.  Now I understand.

I think that the arguments to make-scale should not have implicit steps
(which they currently do).  They should be lists instead of pairs (or else
they should be (step . alteration) pairs.  And the value of each number in
the list should correspond to the weights in the values list.

((0 0 0) (1 0 0) (2 0 0) (3 -1 0) (4 -1 0) (5 -1 0) (6 -1 0) (6 0 0))

(1 1/2 1/4)

That way there is a one-to-one correspondence between the pitch declarations
and the weightings.

As long as we're adjusting things, I think we ought to be consistent.

Thanks,

Carl

[Keith, 2011-02-18 05:43:36]

I advise against changing the internal representation of pitches.

I say that even though this looks like very good and careful code, and after
spending two hours studying it.  The extra complication, for which the care was
given, makes the system more difficult to learn, understand, use, and repair.

Maybe convert the patch to distinguish quarter-tones using the existing Rational
alterations:
  sharp-down = 1/2 - 1/5 = 3/10
  natural-up =    +1/5   = 2/10
These sort in the same order as the solution in this patch.  It should be fine
if the internal representation is not exactly 1/4; some people use the up-arrow
to indicate pitches in just-intonation schemes.  Also, for most instruments
'ces' is not really 1/2 a tone above 'c' but there no harm from representing the
sharp as 1/2.

The email chain linked in the patch mentions a similar method, saying the
'problem' was that method was that transposing from the key of C to the key of
C-natural-up fails to convert a natural-up to a sharp.  However we do not ask
Lilypond to convert 'fisis' to 'g' in a\transpose.  Also, the proposed
representation in this patch has the same 'problem'.  

Transposition with automatic conversion (of 2/10 + 2/10 to 1/2, for example)
should be done by a function distinct form the current \transpose, possibly
written in Scheme using Mike Ellis' modal transposition as a model.

[Graham Percival (old account), 2011-02-18 05:58:28]

On 2011/02/18 05:43:36, Keith wrote:
> The extra complication, for which the care was
> given, makes the system more difficult to learn, understand, use, and repair.

Unfortunately, I believe that the extra complication is necessary.  The commit
message didn't clarify the background or our previous discussions on the topic,
but it boils down to composers wanting to distinguish between cis+1/4 and
cisis-1/4 

On a personal note, I am completely uninterested in microtonal notation; I don't
even care about the difference between equal temperament and just intonation. 
But on a "professional" note, we have heard repeated requests from composers "in
the field" -- including people who make a living from composing and/or music
academia -- so I consider it a very valid request.  Given the multiple long
discussions about the topic, I don't think we can dismiss it.

NB: most requests were for *arbitrary* tunings, such as 31-tone or IIRC even
78-tone.  As far as I understand microtonal composers and their academic
circles, mere quarter-tones is far too mainstream to even consider.  :)

[gbreed, 2011-02-18 06:24:29]

Graham Percival <graham@percival-music.ca> wrote:
> On 2/17/11, Carl.D.Sorensen@gmail.com
> <Carl.D.Sorensen@gmail.com> wrote:
> > I have one question, I think.  Right now, the
> > alteration consists of two integers, which have implied
> > denominators of 1/2 and 1/4, if I understand correctly.
> >
> > Would it be more general to have the alteration consist
> > of two rationals?  Or could we have some way of
> > defining a base-alteration pair that would be
> > rationals?  And then the implied denominators of 1/2
> > and 1/4 would no longer be implicit, but would be
> > explicit in either the alteration or the
> > base-alteration?

It should be two rationals, yes.  There are cases where you
really do want to equally divide the basic accidental, like
with one interpretation of quartertones.  There are just
intonation systems where a bunch of different alterations
might be used, and even monkey-patched by the user.  It's
always possible to multiply through by the lowest common
denominator to get an equal tempered framework, but why do
that?  Rationals are working very well.

The re-scaling of the alterations is interesting but
unnecessary.  They could still be rationals.

It'd be more general to have an arbitrary number of
rationals.  But two is enough to solve the problem here
and some others.

> > I think (but am not sure) that such an implementation
> > would meet the needs that Hans Aberg has defined for
> > appropriate microtonal support.
> 
> Very good point!  I've cc'd Hans and Graham Breed to
> inquire about this.

I have been working with LilyPond and tuning the past few
days.  I wasn't following this though.  I don't have my own
copy of the source so I'm going by this:

> Current patch is here:
> http://codereview.appspot.com/3789044/

It's disruptive to re-define ly:make-pitch and
ly:alteration.  I'm not sure how backwards compatiblity
gets handled.  I work with include files and they aren't
labeled with the version.  Hopefully the files that include
them can be given a version that keeps things working, but
it means a certain version number will be known to break
them.

I don't see why ly:make-pitch can't take a single rational
as the alteration, and assign it as the first alteration in
the majority of cases where that's what you want.  For the
other alteration you can either use an optional argument or
supply a pair.  Maybe either would be considered evil and I
should trust the backwards compatibility scripts, I don't
know.

There are cases where what you want to know is the total
alteration, in standard units relative to the default
scale.  Logically, ly:pitch-alteration should keep on
returning that with an alternative function to return the
pair of alterations that you'd want in different contexts.

There are various snippets, for things like chromatic
transposition, that use ly:make-pitch and
ly:pitch-alteration.  They'll have to be updated for this.
The change doesn't have to be that disruptive.

This is still basically an advance.  I'd have to re-write a
load of code, but it would end up cleaner.  There are
some outstanding problems I think it would help me solve.
There's nothing it breaks that I couldn't work around.

The change to ly:set-default-scale is interesting, but
orthogonal to the problem of ambiguous quartertones.  Does
it mean the acoustic size of the octave can be re-defined?
That would be good.  It's the main limitation of the
current pitch system.  The fixed 12 note MIDI octave is a
problem when you work with alternative tuning tables.  It
happens to be a problem I've discovered how to work around
but it'd be easier not to have to.

The alteration sizes aren't part of the default scale.
It's counter intuitive to set them in that function.


                     t'other Graham

[hanwenn, 2011-02-18 10:18:47]

On Fri, Feb 18, 2011 at 3:43 AM,  <k-ohara5a5a@oco.net> wrote:
> I advise against changing the internal representation of pitches.

I am rather skeptic against patches that would want to do this, but
from a casual glance, it looks thorough at least, and I understand
that the current system is too limited still.  What makes me
uncomfortable that this looks like a non-backward compatible change.

Why not use a sequence of Rationals (rather than ints) to represent
the alteration?  You could maintain the current system in a backward
compatible way (using 1/2 and 1/4 as the only element in the
alteration sequence) or have a system with tuples: +1/2 for the first
element and +1/4 for the second element.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[hanwenn, 2011-02-18 10:21:07]

On Fri, Jan 28, 2011 at 12:17 PM,  <percival.music.ca@gmail.com> wrote:
> Hi Felipe,
>
> I'm very sorry about the delay, but at least I'm looking at it now, and
> I'll take care of badgering people to review it once it's ready.
>

Nitpick: if this were called something like "change internal pitch
representation" in the first line of the patch, I would have looked
earlier. From a casual glance at my mailbox, it looked like it would
be a simple bugfix patch for issue 1278.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[gbreed, 2011-02-18 11:20:04]

The obvious workaround to distinguish cis+1/4 and cis-1/4 is to replace 1/4 with
something like 9/40.  That fails because if you transpose a quartertone up by a
quartertone, you expect to get a semitone.  That is, a regular sharp.  Having
two ways to specify the same fractional alteration is a solution to this
problem.

A simple way for it to work is for the accidental glyph assigner to fall back to
using the overall alteration when the pair fails to match.  That would also
preserve backwards compatibility because code that doesn't care about
distinguishing the different same-pitch alterations can deal with the overall
alteration.

Unfortunately, that doesn't seem to be what's happening here.  I can't find
where transpositions are dealt with.  There are a load of places that hard-code
the regular half-sharp as (0 . 1) and a full-sharp as (1 . 0).  For this to
work, you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0).  Where is that
specified?  How do we change it for other tunings?  The worst that can happen is
(0 . 1) + (0 . 1) = (0 . 2) and the result is not recognized as a regular sharp.
 That's exactly the problem the patch is supposed to fix.

The description has this subtly scary "The Pitch class was reimplemented. Its
interface was mostly preserved."  Unless I'm missing something, this "mostly
preserved" means changing the meaning of two functions instead of creating new
ones.  It means the internal representation bleeds into exactly the functions
we're using for manipulating pitches in things like custom transpositions.  This
will break existing files.  It will break snippets in the standard
documentation.

(If anybody comes to you asking for arbitrary tunings, send them to me.  Most
things work but they entail hacking up an include file.)

[Keith, 2011-02-18 21:46:35]

On 2011/02/18 11:20:04, gbreed wrote:
> The worst that can happen is
> (0 . 1) + (0 . 1) = (0 . 2) and the result is not recognized as a regular
sharp.
>  That's exactly the problem the patch is supposed to fix.

The request on the tracker
(http://code.google.com/p/lilypond/issues/detail?id=1278) asks for the internal
representation to keep quarter tones distinct from semitones.  Therefore cihih
(0 . 2) is kept distinct from 'cis' (1 . 0).

> you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0). 

For the old system, the snippet http://lsr.dsi.unimi.it/LSR/Snippet?id=266 (also
in the Notation Reference, after \transpose) defines \naturalizeMusic to perform
the analogous reduction of 'cisis' to 'd'.  Maybe it could be adapted to work
with this patch, but I don't see how.

In the background email chains, people seem to like the solution using
not-exactly-1/4 for the quarter tones, except they want to reduce ihih to is
when appropriate.  \naturalizeMusic could have a round-to-nearest-1/2 added to
the end, to convert 2*(not-exactly-1/4) to 1/2.

On 2011/02/18 10:18:47, hanwenn wrote:
> Why not use a sequence of Rationals [..] to represent the alteration? 

A single Rational can hold the series as a sum, and preserve the separability of
the terms, if we use mutually prime denominators.  
In scales where cih is identical to ciseh, ciseh can be +1/2 - 1/4 = 1/4.
In scales where cih is logically distinct from ciseh, ciseh can be +1/2 - 1/5.

[Carl, 2011-02-18 22:03:49]

On 2011/02/18 11:20:04, gbreed wrote:

> Unfortunately, that doesn't seem to be what's happening here.  I can't find
> where transpositions are dealt with.  There are a load of places that
hard-code
> the regular half-sharp as (0 . 1) and a full-sharp as (1 . 0).  For this to
> work, you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0).  Where is that
> specified?  How do we change it for other tunings?  

In the current patch, this is specified by the weightings, where the weighting
of the first number in the pair is 1/2 and the weighting of the second number is
1/4.  You can only see this in scm/lily.scm, where global-scale is defined. 
IMO, this is not a good place to have it buried.

[Keith, 2011-02-18 22:11:46]

On 2011/02/18 22:03:49, Carl wrote:
> > work, you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0).  
> 
> In the current patch, this is specified by the weightings, where the weighting
> of the first number in the pair is 1/2 and the weighting of the second number
is
> 1/4. 

But there is no conversion from (0 . 2) to (1 . 0).  Also, the comparison
operators report (0 . x) < (1 . 0) for all x, all of which is consistent with
enhancement request 1278.

[gbreed, 2011-02-19 08:50:25]

On 2011/02/18 21:46:35, Keith wrote:

> The request on the tracker
> (http://code.google.com/p/lilypond/issues/detail?id=1278) asks for the
internal
> representation to keep quarter tones distinct from semitones.  Therefore cihih
> (0 . 2) is kept distinct from 'cis' (1 . 0).

The request doesn't address this issue that I can see.  All it says is that the
same pitch (on the same scale degree) should be written two different ways.  It
also doesn't mention the workaround of making them slightly different pitches,
let alone give a practical reason why it's inadequate.  In summary, it doesn't
make a case for changing the pitch representation.

Note: the attached document describing the notation doesn't say the quartertones
should be exactly 50 cents.  It makes the interesting comparison "to the
enharmonics of traditional notation".  Well, traditionally they weren't the same
pitch, and aren't necessarily the same pitch in Lilypond.

> > you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0). 
> 
> For the old system, the snippet http://lsr.dsi.unimi.it/LSR/Snippet?id=266
(also
> in the Notation Reference, after \transpose) defines \naturalizeMusic to
perform
> the analogous reduction of 'cisis' to 'd'.  Maybe it could be adapted to work
> with this patch, but I don't see how.

Sure it can be adapted.  But is the pain of adapting all these snippets greater
than the pain of changing the pitch representation and important parts of its
interface?

[hanwenn, 2011-02-19 13:00:17]

On Fri, Feb 18, 2011 at 7:46 PM,  <k-ohara5a5a@oco.net> wrote:

>> Why not use a sequence of Rationals [..] to represent the alteration?
>
> A single Rational can hold the series as a sum, and preserve the
> separability of the terms, if we use mutually prime denominators.
> In scales where cih is identical to ciseh, ciseh can be +1/2 - 1/4 =
> 1/4.
> In scales where cih is logically distinct from ciseh, ciseh can be +1/2
> - 1/5.

I know that, but

1. The separability will go out the window if people start transposing
by these amounts.
+1/1 - 1/5 = 3/10.   Transpose by that 5 times, and you have 15/10 =
3; the quarter tones have gone.

I realize this concern is mostly academic.

2. If what we really want to represent is a sequence of numbers (and I
gather it is), we should make the internal representation a sequence
too.  If we use rational numbers, we can maintain backward
compatibility.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[felipeg.assis_gmail.com, 2011-02-19 18:59:32]

Hello everyone,

First of all, thanks for the extensive comments made on this. I will try to
address the major topics in this mail. Sorry if I miss anything.

=======
1. Issue header

> Nitpick: if this were called something like "change internal pitch
> representation" in the first line of the patch, I would have looked
> earlier. From a casual glance at my mailbox, it looked like it would
> be a simple bugfix patch for issue 1278.

I should remind you that this patch was at first only casually posted as a
background for a discussion I started by the end of the last year, see
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00726.html

I am taking this review as a continuation of that discussion. So, for those
of you who did not follow that, I will give relevant pointers below. For
now, I think you really should check the attachments in
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00730.html

I will occasionally make references to towards123, which is attached in
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00729.html
-----

2. int vs Rational

> Why not use a sequence of Rationals (rather than ints) to represent
> the alteration?

> If we use rational numbers, we can maintain backward compatibility.

> The re-scaling of the alterations is interesting but
> unnecessary.  They could still be rationals.

That is a very reasonable idea. Its main advantage would be, as pointed, in
more robust backwards compatibility, as well as ease of implementation.

If you decide for that, I can work on a new patch.

Further considerations of mine might be found below question 1.1 in
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00729.html
and after the last quote in
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00737.html
-----

3. Transposition

> you have to set a rule that (0 . 1) + (0 . 1) = (1 . 0).

No, we don't. Transposing c to cis takes cis to cisis, not to d.

If you are not using arrow notation, you can define a quarter tone to be
(1 . 0).

Also, see the next topic, for further discussion.

-----

4. Normalisation

Many objections regarding transposition in my current patch, or in
LilyPond's current behaviour, are related to what I am calling
normalisation. This is what may cause musical transposition to look
complicated, it really is not.

My considerations on the subject can be found in Section 3 of towards123.
-----

5. Scale 1

> I think that the arguments to make-scale should not have implicit steps
> (which they currently do)

Steps _mean_ position on Scale, which in its turn means position in the
staff. I assume this to uniquely determine one of the coefficients in
the group representation of the pitch. The vector argument to make-scale
then just specifies what alteration is attached to each position when no
accidental is added to it.

This well models every notation system I have come across. Besides, a
notation system in which this did not work would either need some specific
algorithm to determine the staff position of a pitched note, or not work
with transpositions well.
-----

6. Scale 2

> The alteration sizes aren't part of the default scale.
> It's counter intuitive to set them in that function.

Firstly, the default scale effectively is just the static part of the C++
Pitch class, except when you instantiate a Pitch, then reset the default
scale, and then use that Pitch (which will refer to the old scale).

Secondly, see my considerations on Section 1 of towards123.
-----

7. Default arguments in ly:make-pitch

> I don't see why ly:make-pitch can't take a single rational
> as the alteration, and assign it as the first alteration in
> the majority of cases where that's what you want.

Whether we to decide for ints or for Rationals, I completely agree that
make-pitch should have optional arguments. Even the alteration itself
should be optional.

The single reason why I chose not to enable that in this patch was to track
parts of the code that would need rewriting, by making the code break until
I edited them.

I was particularly concerned with the commits made after my patch, that
could silently invalidate it.

Of course, none of this would have to be undertaken with a Rational
pair/list representation.
=======

If you have any further questions, or if you think that I forgot something,
please write.

Also:
Should I write a new version of this patch using Rational instead of int?

I look forward to hearing from you.

Regards,
Felipe

[Keith, 2011-02-20 21:34:34]

On 2011/02/18 04:35:00, c_sorensen_byu.edu wrote:
> I think that the arguments to make-scale should not have implicit steps
> (which they currently do). [...]
> 
> ((0 0 0) (1 0 0) (2 0 0) (3 -1 0) (4 -1 0) (5 -1 0) (6 -1 0) (6 0 0))
> 
> (1 1/2 1/4)
> 

The final pitch shouldn't be there (nor should the one in makam.ly).
However, it makes a convenient illustration Felipe's point about "steps mean
position on the staff".  The last element would be (7 -2 0) to indicate that
transposing from b(6 -1) to c'(7 -2) means: raise a tone and lower a semitone.

Using Felipe's system, any sensible scale would have the steps increase
one-by-one.
Nevertheless, I agree with Carl that the step should be explicit in make-scale. 
People unfamiliar with the system need to see the complete picture in one place.

[Keith, 2011-02-20 21:40:17]

On 2011/02/19 13:00:17, hanwenn wrote:
> > In scales where cih is logically distinct from ciseh, ciseh can be +1/2
> > - 1/5.
> 
> I know that, but
> 
> 1. The separability will go out the window if people start transposing
> by these amounts.

True.  That representation was intended for the application behind the
bug-tracker report, where separability was desired for single 'eh', but not for
any 'eheh' produced by \transpose. I suggested (too late) a normalize step after
transpose to convert 2/5 to a semitone.
(http://lists.gnu.org/archive/html/lilypond-user/2009-04/msg00649.html
 http://lists.gnu.org/archive/html/lilypond-user/2011-02/index.html)

For just-intonation schemes, different alteration-types keep their separation so
long as the representation of pitch is sufficiently accurate.  For example, an
"extended Helmholz-Ellis" alteration with B sharps/flats, C arrows, and D hooks
is (B * 114/200 + C * 22/200 + D * 27/200) in terms of the current system.

> 2. If what we really want to represent is a sequence of numbers (and I
> gather it is), we should make the internal representation a sequence
> too.  If we use rational numbers, we can maintain backward
> compatibility.

Yes. It seems that the various ways to express alterations can be represented by
a list of rationals.

Two alterations are usually considered distinct if_and_only_if the corresponding
sequences have different sums.  So, a single-element-list will handle the common
cases.  A single Rational is certainly what we want for equal-temperaments, so
that in 72ET raising a 1/12 by 1/12 gives 1/6, rather than keeping 2/12
distinct.

[gbreed, 2011-02-21 10:36:10]

On 2011/02/20 21:40:17, Keith wrote:

> For just-intonation schemes, different alteration-types keep their separation
so
> long as the representation of pitch is sufficiently accurate.  For example, an
> "extended Helmholz-Ellis" alteration with B sharps/flats, C arrows, and D
hooks
> is (B * 114/200 + C * 22/200 + D * 27/200) in terms of the current system.

Extended Helmholtz-Ellis can be, and to an extent has been, implemented in the
current system.  But it's cumbersome because you have to define a huge number of
potential alterations that may (I don't know if this is a problem in practice)
slow down the system.  You can also build accidental strings through tweaks and
overrides.  A list of alterations would make it all more straightforward, should
that be on the table.