Add Modal transformations

Add Modal transformations

 - as provided by Michael Ellis <michael.f.ellis@gmail.com>

[Trevor Daniels, 2011-01-31 10:26:44]

The set of modal transforms provided by Mike Ellis are now ready for review.  As
the changes are all in Scheme code they are easy to test. 

Patch set 1 are as originally provided by Mike; patch set 2 are after some
tidying up by me.

As yet there is no documentation.  I'll begin writing that now, but I'd prefer
to see comments on the code before I get too far into that.

Trevor

[Trevor Daniels, 2011-02-01 16:34:28]

First stab at documentation now added.  Comments from anyone familiar with these
operations welcome.

Trevor

[Graham Percival (old account), 2011-02-01 23:15:34]

Might it be worth having some predefined scales, i.e. \diatonicScale and
\pentatonicScale and the like?

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:823: In a musical composition that is based
on a scale, a motive is frequently transformed in various ways.
wrap at 72 chars, please.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:824: It may be @emph{transposed} to start
at different
No @emph here, since the reader already knows about tranposition.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:825: places in the scale, it may be
@emph{inverted} around
Technically, this should be @notation{inverted}, since it's a musical term.  The
output is the same, but using @notation instead would let us theoretically
change the display of all notation terms at once, if we had some reason to do
so.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:828: Lilypond provides functions that may
be used to apply these transformations to a motive.
I don't think we need a whole paragraph-sentence for this.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:831: Any note that does not is left
untransformed.}
This could be rewritten with half the number of words, and a clearer single
sentence.  And since it's inside a @warning, I'm going to insist on it.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:838: @funindex \modalTranspose
at the moment, we're writing out (elsewhere in the docs)
funindex \foo
funindex foo

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:840: A motive can be transposed within a
given scale.  The syntax is
Remove "the syntax is".

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:851: motive = \relative c' { c8 d e f g a b
c }
I've never seen it spelled that way; I've always seen "motif".

I'm not saying that this is necessarily wrong, but please double-check.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:862: Any scale may be specified.  Here is
an example with a pentatonic scale:
Remove "Here is an example..."

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:871: \modalTranspose ges ees'
\blackNoteScale \motive
WTM is a "black note scale"?  I'm a cellist.

What's wrong with \pentatonic?  or pentatonicScale ?

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:899: A motive can be inverted within a
given scale around a given pivot note.  The syntax is
Remove "the syntax is"

[Trevor Daniels, 2011-02-02 11:20:22]

> Might it be worth having some predefined scales, 
> i.e. \diatonicScale and \pentatonicScale and the like?

I don't think so; they are not unique.  Even a chromatic scale can be written
using many combinations of naming the enharmonically equivalent notes.  You
might argue that a C major scale is somehow more basic, but that's only because
it corresponds with the white notes on a piano ;)

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:823: In a musical composition that is based
on a scale, a motive is frequently transformed in various ways.
> wrap at 72 chars, please.

Done

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:824: It may be @emph{transposed} to start
at different

> No @emph here, since the reader already knows about
> tranposition.

As this sentence is introducing the three transformations I want them all to
have the same appearance, so I changed them all to notation{}

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:825: places in the scale, it may be
@emph{inverted} around
> Technically, this should be @notation{inverted},

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:828: Lilypond provides functions that may
be used to apply these transformations to a motive.
On 2011/02/01 23:15:34, Graham Percival wrote:

> I don't think we need a whole paragraph-sentence for this.

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:831: Any note that does not is left
untransformed.}
On 2011/02/01 23:15:34, Graham Percival wrote:
> This could be rewritten with half the number of words,
> and a clearer single sentence.  And since it's inside a
> @warning, I'm going to insist on it.

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:838: @funindex \modalTranspose
On 2011/02/01 23:15:34, Graham Percival wrote:
> at the moment, we're writing out (elsewhere in the docs)
> funindex \foo
> funindex foo

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:840: A motive can be transposed within a
given scale.  The syntax is
On 2011/02/01 23:15:34, Graham Percival wrote:

> Remove "the syntax is".

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:851: motive = \relative c' { c8 d e f g a b
c }
Good catch!  Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:862: Any scale may be specified.  Here is
an example with a pentatonic scale:
On 2011/02/01 23:15:34, Graham Percival wrote:

> Remove "Here is an example..."

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:871: \modalTranspose ges ees'
\blackNoteScale \motive
On 2011/02/01 23:15:34, Graham Percival wrote:

> What's wrong with \pentatonic?  or pentatonicScale ?

Done.

http://codereview.appspot.com/4126042/diff/4002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:899: A motive can be inverted within a
given scale around a given pivot note.  The syntax is
On 2011/02/01 23:15:34, Graham Percival wrote:

> Remove "the syntax is"

Done.

[bernard_marcade.biz, 2011-02-02 17:55:06]

On Wed, Feb 02, 2011 at 11:20:22AM +0000, tdanielsmusic@googlemail.com wrote:
>
>
>> Might it be worth having some predefined scales,
>> i.e. \diatonicScale and \pentatonicScale and the like?
>
> I don't think so; they are not unique.

I'm not sure what you mean by this. For instance:

"\majorScale c" is essentially:
{c d e f g a b c}

whereas "\majorScale es" is that transposed to es:
{es f g as bes c d es}

"\majorScale" itself actually represents a sequence of intervals but it could
equally well be represented by the scale on c.

Incidentally I am in the middle of a project to sysematically name all
seven-note scales, none of whoes modes contain a sequence of either more than
two consecutive semitones or more than two consecutive aumented seconds.

  /Bernard

[Graham Percival, 2011-02-02 19:38:49]

On Wed, Feb 02, 2011 at 05:55:04PM +0000, Bernard Hurley wrote:
> On Wed, Feb 02, 2011 at 11:20:22AM +0000, tdanielsmusic@googlemail.com wrote:
> >> Might it be worth having some predefined scales,
> >> i.e. \diatonicScale and \pentatonicScale and the like?
> >
> > I don't think so; they are not unique.
> 
> I'm not sure what you mean by this. For instance:

IIRC, "diatonic" can refer to any church mode.

Let me rephrase / alter my initial suggestion: might it be worth
having some predefined scales for actually well-defined scales?
Like \major or \locrian or the like?  They could go in a new
ly/*-init.ly file, or maybe something in scm/.  Something like
"define-scales-init.ly" ?

For octatonic, we could have octatonicA and octatonicB, or
something like that... IIRC there's only two types of those
scales.  (or maybe we should avoid using A and B, and instead call
them "octatonicBeginFull" and "octatonicBeginHalf" ? )

Cheers,
- Graham

[Keith, 2011-02-03 07:22:36]

This works very nicely for me. 
One tiny oops, and suggestions you can take or leave as you wish.

I think I understand the point that some scales *can* un-ambiguously named, but
a facility to generate scales from these names is not necessary for this patch.

I can type
 myPentatonic = \relative c' { a b d e fis }
just as easily as
 myPentatnoic = \majorPentatonicScale d

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:867: pentatonicScale = \relative c' { ges
aes bes des ees ges }
Oops, you want only the first /five/ pitches.
If you like, you could simplify by basing the scale on f 
 \relative c' { f g a c d }
\motif = \relative c' { d8 c f,4 <f' a,> <f a,> }

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:943: motif = \relative c' { c8. ees16 fis8.
a16 b8. gis16 f8. d16 }
Optionally, you could throw in a slur or crescendo or some other spanner.  Users
might otherwise assume the retrograding will break slurs and do un-necessary
work to put slurs in another variable.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:967: 
Optionally, a note that ties break, even though slurs work fine.  (Internally,
ties are only attached to one note)
@knownissues
Manual ties inside @code{\retrograde} will be broken and
generate warnings.  @ref{Automatic note splitting} can be
enabled to generate some ties automatically.

http://codereview.appspot.com/4126042/diff/5002/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/4126042/diff/5002/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:465: modalInversion =
Since it is an operator, should it be a verb, modalInvert ?
The distinction between \transpose and \transposition is so tricky, that I think
it worth being careful in similar items.
\modalTranspose is the right name because it is an operation as is \transpose

http://codereview.appspot.com/4126042/diff/5002/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):

http://codereview.appspot.com/4126042/diff/5002/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:157: ;; 5 octaves from the original scale definition. 
In practice there
It's not even that bad. If the scale is defined near middle C, we can go from
five-ish octaves below middle C to five-ish octaves above.  If I move the scale
away from my music I get the "pitch not in scale" message, which I think is very
reasonable.

[Keith, 2011-02-03 07:44:37]

http://codereview.appspot.com/4126042/diff/5002/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):

http://codereview.appspot.com/4126042/diff/5002/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:157: ;; 5 octaves from the original scale definition. 
In practice there
On 2011/02/03 07:22:36, Keith wrote:
Oh! Now I realize that I just restated exactly what Mike's comment said.
The word "hack" made it sound bad, but I think it's not bad at all.

[bernard_marcade.biz, 2011-02-03 08:24:17]

On Wed, Feb 02, 2011 at 07:38:45PM +0000, Graham Percival wrote:
> 
> Let me rephrase / alter my initial suggestion: might it be worth
> having some predefined scales for actually well-defined scales?
> Like \major or \locrian or the like?  They could go in a new
> ly/*-init.ly file, or maybe something in scm/.  Something like
> "define-scales-init.ly" ?
> 

We could add things like the Messiaen modes.

It would also be nice if we could define our own scales as in:

\defineScale fred = {c d es fis g as bes c}

One of my favourite scales. I don't know if this syntax would be best but I
think it is clear what I mean.

Would it be possible to support scales that repeat at intervals other than an
octave? As, for instance, Hauer and Bartok sometimes used.


> For octatonic, we could have octatonicA and octatonicB, or
> something like that... IIRC there's only two types of those
> scales.  (or maybe we should avoid using A and B, and instead call
> them "octatonicBeginFull" and "octatonicBeginHalf" ? )
> 

Personally I don't like things like "A", "B", "One", "Two" unless they are part
of an established name.

  /Bernard

[pkx166h, 2011-02-03 10:14:46]

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:828: Lilypond provides functions for
applying these transformations.
Do we need this last sentence 'Lilypond provides..'.

If so 'LilyPond' not 'Lilypond'.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:862: @end lilypond
This maybe showing my ignorance here, but do we need the '\score' and/or '\new
Staff' between the @Lilypond and @end Lilypond constructs? - for the sake of
simplifying the examples.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:868: motif = \relative c' { ees8 des ges,4
<ges' bes,> <ges bes,>}
Minor nitpick. need a space before the closing '}'

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:912: opposite direction:
I had to read this a few times for to make sense to me, so how about instead:

The notes of @var{motif} are placed the same number of scale degrees from the
@var{pivot} note within the @var{scale}, but in the opposite direction.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:953: The combined operation of inversion
and retrograde produce the
The combined operation of @code{inversion} and @code{retrograde} ...

[Trevor Daniels, 2011-02-03 10:25:23]

Thanks Keith

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:867: pentatonicScale = \relative c' { ges
aes bes des ees ges }
On 2011/02/03 07:22:36, Keith wrote:

> Oops, you want only the first /five/ pitches.
Done

> If you like, you could simplify by basing the scale on f 
>  \relative c' { f g a c d }
> \motif = \relative c' { d8 c f,4 <f' a,> <f a,> }

This was originally called blackNoteScale.  I think I prefer
to leave it as the black notes so the example retains its familiarity.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:943: motif = \relative c' { c8. ees16 fis8.
a16 b8. gis16 f8. d16 }
On 2011/02/03 07:22:36, Keith wrote:

> Optionally, you could throw in a slur
Done.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:967: 
On 2011/02/03 07:22:36, Keith wrote:

> Optionally, a note that ties break, even though slurs 
> work fine.  (Internally, ties are only attached to one
> note)
> @knownissues
> Manual ties inside @code{\retrograde} will be broken and
> generate warnings.  @ref{Automatic note splitting} can be
> enabled to generate some ties automatically.
Done

[t.daniels_treda.co.uk, 2011-02-03 11:00:39]

Keith wrote Thursday, February 03, 2011 7:22 AM

>
http://codereview.appspot.com/4126042/diff/5002/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:465: modalInversion =
> Since it is an operator, should it be a verb, modalInvert ?
> The distinction between \transpose and \transposition is so 
> tricky, that
> I think it worth being careful in similar items.
> \modalTranspose is the right name because it is an operation as is
> \transpose

Inadvertently I omitted to respond to this on Rietveld.

While I take your point, \modalInvert doesn't quite sound
right to me, and that makes it harder to remember.  The
modifier should really be adverbial, \modallyInvert, if
a verbal form is to be used.  That's a bit longer though.
And if we make that change I guess we should use
\modallyTranspose too, which is even longer.

So I'm undecided and made no change at this time.  Let's
see if any other opinion is expressed first.

Trevor

[Trevor Daniels, 2011-02-03 11:58:18]

Thanks James

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:828: Lilypond provides functions for
applying these transformations.
On 2011/02/03 10:14:47, pkx166h wrote:

>Do we need this last sentence 'Lilypond provides..'.

No.  Deleted.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:862: @end lilypond
On 2011/02/03 10:14:47, pkx166h wrote:

> do we need the '\score' and/or '\new Staff' between the
> @Lilypond and @end Lilypond constructs?

Yes.  It's the same with \transpose.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:868: motif = \relative c' { ees8 des ges,4
<ges' bes,> <ges bes,>}
On 2011/02/03 10:14:47, pkx166h wrote:

>need a space before the closing '}'
Done.

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:912: opposite direction:
On 2011/02/03 10:14:47, pkx166h wrote:

> I had to read this a few times for to make sense to me,
> so how about instead:
Done

http://codereview.appspot.com/4126042/diff/5002/Documentation/notation/pitche...
Documentation/notation/pitches.itely:953: The combined operation of inversion
and retrograde produce the
On 2011/02/03 10:14:47, pkx166h wrote:

> The combined operation of @code{inversion} and 
> @code{retrograde} ...
No.  These are not code items.  If anything they might be
@notation{}, but we don't do that for every musical term
every time it is used, only the first time in a section.

[t.daniels_treda.co.uk, 2011-02-03 12:43:22]

Graham Percival wrote Wednesday, February 02, 2011 7:38 PM


> IIRC, "diatonic" can refer to any church mode.
>
> Let me rephrase / alter my initial suggestion: might it be worth
> having some predefined scales for actually well-defined scales?
> Like \major or \locrian or the like?  They could go in a new
> ly/*-init.ly file, or maybe something in scm/.  Something like
> "define-scales-init.ly" ?

Well, we could, but each scale would be specific to a single
key.  We could choose to define \major in the key of C and
expect users to use \transpose to generate scales in other
keys, but as Keith pointed out that is hardly shorter than
writing out the desired scale anyway.  And what key would you
chose for \minor?  Am or Cm?  And to distinguish the several
minor scales the names would need to be longer still.

The scales of the various "church" modes appear to have several
variants, apart from being key-specific, although I confess to
being pretty ignorant here.

For pentatonic scales there are even more in common use,
I believe.  Even the five black-key scales are all distinct.

> For octatonic, we could have octatonicA and octatonicB, or
> something like that... IIRC there's only two types of those
> scales.  (or maybe we should avoid using A and B, and instead call
> them "octatonicBeginFull" and "octatonicBeginHalf" ? )

These are modes within any one of the three distinct diminished
octotonic scales.  But in total there are 42 other 
non-enharmonically
equivalent octotonic scales.

So I wonder if this is worth the trouble.  AFAIK there is no
universal naming convention to distinguish them all.  As
Bernard says, he is engaged in systematically naming all the
seven-note scales, and even this is with limitations.

Trevor

[benko.pal, 2011-02-03 15:28:51]

>> IIRC, "diatonic" can refer to any church mode.
>>
>> Let me rephrase / alter my initial suggestion: might it be worth
>> having some predefined scales for actually well-defined scales?
>> Like \major or \locrian or the like?  They could go in a new
>> ly/*-init.ly file, or maybe something in scm/.  Something like
>> "define-scales-init.ly" ?
>
> Well, we could, but each scale would be specific to a single
> key.  We could choose to define \major in the key of C and
> expect users to use \transpose to generate scales in other
> keys, but as Keith pointed out that is hardly shorter than
> writing out the desired scale anyway.  And what key would you
> chose for \minor?  Am or Cm?  And to distinguish the several
> minor scales the names would need to be longer still.
>
> The scales of the various "church" modes appear to have several
> variants, apart from being key-specific, although I confess to
> being pretty ignorant here.

I think for these purposes all church modes are equivalent.
the different minor scales are truly different.

> For pentatonic scales there are even more in common use,
> I believe.  Even the five black-key scales are all distinct.

but are equivalent, just as the church modes are equivalent
for diatonic transpositions and mirroring.

(I'm waiting eagerly for these features, no matter whether there
are preinstalled definitions in -init.ly files or not; they are a lot nicer
than my hacks for diatonic mirroring!)

p

[t.daniels_treda.co.uk, 2011-02-03 17:39:55]

Benkő Pál wrote Thursday, February 03, 2011 3:28 PM

> I think for these purposes all church modes are equivalent.
> the different minor scales are truly different.

Don't some church modes have an optional flat?

>> For pentatonic scales there are even more in common use,
>> I believe. Even the five black-key scales are all distinct.
>
> but are equivalent, just as the church modes are equivalent
> for diatonic transpositions and mirroring.

Of course, you're right about the black-key scales, which
is the pentatonic scale I've used.  But they can still be
defined with different note names.  Which would we choose?

> (I'm waiting eagerly for these features, no matter whether there
> are preinstalled definitions in -init.ly files or not; they are a 
> lot nicer
> than my hacks for diatonic mirroring!)

I think the revision is pretty well complete now.  If
there are no further comments I'll push this on Saturday.
All the comments have been on the documentation anyway.
Mike's code looks good.

Whatever we decide about defining some scales I'd want
to make that a separate patch anyway, so we don't have
to wait for this discussion to crystallise.

Trevor

[benko.pal, 2011-02-03 20:14:32]

>> I think for these purposes all church modes are equivalent.
>> the different minor scales are truly different.
>
> Don't some church modes have an optional flat?

that's the concern of musica ficta, no need to handle it here.

>>> For pentatonic scales there are even more in common use,
>>> I believe. Even the five black-key scales are all distinct.
>>
>> but are equivalent, just as the church modes are equivalent
>> for diatonic transpositions and mirroring.
>
> Of course, you're right about the black-key scales, which
> is the pentatonic scale I've used.  But they can still be
> defined with different note names.  Which would we choose?

a c d e g (since it's symmetric; for the same reason my choice
for diatonic scale is a b c d e f g).

p

[Graham Percival (old account), 2011-02-04 01:43:38]

LGTM.  I don't think you need to wait until Sat.

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:840: A motif can be transposed within a
given scale with
colon please:

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:851: diatonicScale = \relative c' { c d e f
g a b }
(incidentally, only my first reading of the patch, I didn't notice that stuff
like \diatonicScale was created in the examples; I thought they were getting
defined in a scm file or something.  If I'd noticed that, I wouldn't have
bothered nitpicking about the names)

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:903: note with
Colon please:

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:933: A motif can be reversed to produce its
retrograde with
ditto:

although in this case, I really don't think we need an @example.  I mean,
  \retograde \music-expression
is pretty clear.

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:972: Manual ties inside @code{\retrograde}
will be broken and
not a patch comment, but I'm intrigued -- how often do you think that people use
automatic ties?  And why does this code work with the Completion_heads_engraver
but not manual ties?

http://codereview.appspot.com/4126042/diff/3009/input/regression/modal-transf...
File input/regression/modal-transforms.ly (right):

http://codereview.appspot.com/4126042/diff/3009/input/regression/modal-transf...
input/regression/modal-transforms.ly:9: ges' aes' a' b'
heh, I thought that after so many people reviewing it, there'd be nothing
"substantial" left to comment on.

missing duration:
  ges'4
:)


oh wait, this is a regtest, not the docs.  ok, never mind, we're not fussy about
input code syntax here.

[benko.pal, 2011-02-04 08:31:13]

>
http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
> Documentation/notation/pitches.itely:972: Manual ties inside
> @code{\retrograde} will be broken and
> not a patch comment, but I'm intrigued -- how often do you think that
> people use automatic ties?

in my grand "mensural and modern from same source"
scheme it's always the case.  (And unfortunately retrograde
is more difficult, but that's my problem, not that of the patch.)

> And why does this code work with the
> Completion_heads_engraver but not manual ties?

The engraver processes the music _after_ \retrograde,
so the latter sees correct input.

p

[Neil Puttock, 2011-02-04 10:24:26]

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
Documentation/notation/pitches.itely:914: octotonicScale = \relative c' { ees f
fis gis a b c d }
octatonicScale

http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:467: (ly:music? ly:music? ly:music?)
indent

#(define-music-function
   (ly:music?

http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:475: (ly:music? ly:music? ly:music? ly:music?)
indent

http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:800: (ly:music?)
indent

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:25: ;; Returns a transposer for the specified scale
"docstring"

(same for all the other functions)

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:38: (ly:warning "Root pitch not in scale!")
none of the warnings follow coding style

"root pitch not in scale"

+ need localizing: (_i "warning")

[benko.pal, 2011-02-04 17:11:36]

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:68: (lambda (pivot-pitch pitch)
I'm afraid this is not a good interface, as there are inversions where the pivot
is between two notes.

e.g. with the scale { c g } I may want to invert
{ c4. c'8 g'4. g8 } to get { g4. g,8 c,4. c8 }.

I see two ways out:
1. have a source-pitch and a target-pitch, like transposition
2. have a single pitch, which is not a pivot, but the image of the starting
pitch in the scale definition.

http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
scm/modal-transforms.scm:83: (index pitch scale)))
With my suggestion 1. above these three lines could be written as

(+ (index target-pitch scale)
   (- (index source-pitch scale)
      (index pitch scale)))

[t.daniels_treda.co.uk, 2011-02-05 11:28:40]

Pal wrote Friday, February 04, 2011 5:11 PM

>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
> scm/modal-transforms.scm:68: (lambda (pivot-pitch pitch)
> I'm afraid this is not a good interface, as there are inversions 
> where
> the pivot is between two notes.
>
> e.g. with the scale { c g } I may want to invert
> { c4. c'8 g'4. g8 } to get { g4. g,8 c,4. c8 }.

Good point!

> I see two ways out:
> 1. have a source-pitch and a target-pitch, like transposition
> 2. have a single pitch, which is not a pivot, but the image of the
> starting pitch in the scale definition.

There is a third way, which I would prefer.  An inversion
can pivot around a note in the scale, as now, or around a
point between two adjacent notes in the scale, which is
missing from the current implementation.

I would suggest permitting the pivot to be either one
note, as now, or two notes, in which case the pivot is
a point between the two.

>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
> scm/modal-transforms.scm:83: (index pitch scale)))
> With my suggestion 1. above these three lines could be written as
>
> (+ (index target-pitch scale)
>    (- (index source-pitch scale)
>       (index pitch scale)))

With my suggestion your example would then be coded:

  \modalInversion { c g } {c g} { c4. c'8 g'4. g8 }

@Mike: do you see any problems with this?  Would you be
able to code this up?  I can fix the docs.

> http://codereview.appspot.com/4126042/

Trevor

[Trevor Daniels, 2011-02-05 12:04:07]

I had to start a new branch and Rietveld number.  See

http://codereview.appspot.com/4079064/

for the continuation of this isse.

[t.daniels_treda.co.uk, 2011-02-05 12:09:20]

Changes made in http://codereview.appspot.com/4079064/

----- Original Message ----- 
From: <n.puttock@gmail.com>
Sent: Friday, February 04, 2011 10:24 AM
Subject: Re: Add Modal transformations (issue4126042)


>
http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
> Documentation/notation/pitches.itely:914: octotonicScale = 
> \relative c'
> { ees f fis gis a b c d }
> octatonicScale
Done

>
http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:467: (ly:music? ly:music? ly:music?)
> indent
Done

>
http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:475: (ly:music? ly:music? ly:music?
> ly:music?)
> indent
Done

>
http://codereview.appspot.com/4126042/diff/3009/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:800: (ly:music?)
> indent
Done

>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
> scm/modal-transforms.scm:25: ;; Returns a transposer for the 
> specified
> scale
> "docstring"
>
> (same for all the other functions)
Done

>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
> scm/modal-transforms.scm:38: (ly:warning "Root pitch not in 
> scale!")
> none of the warnings follow coding style
>
> "root pitch not in scale"
>
> + need localizing: (_i "warning")
Done

> http://codereview.appspot.com/4126042/
for continuation see http://codereview.appspot.com/4079064/

Trevor 

[t.daniels_treda.co.uk, 2011-02-05 12:14:16]

Changes in http://codereview.appspot.com/4079064/

From: <percival.music.ca@gmail.com>
Sent: Friday, February 04, 2011 1:43 AM
Subject: Re: Add Modal transformations (issue4126042)


> LGTM.  I don't think you need to wait until Sat.

Perhaps I should have waited :(

>
http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
> Documentation/notation/pitches.itely:840: A motif can be 
> transposed
> within a given scale with
> colon please:
Done

>
http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
> Documentation/notation/pitches.itely:903: note with
> Colon please:
Done

>
http://codereview.appspot.com/4126042/diff/3009/Documentation/notation/pitche...
> Documentation/notation/pitches.itely:933: A motif can be reversed 
> to
> produce its retrograde with
> ditto:
Done

> although in this case, I really don't think we need an @example. 
> I
> mean,
>   \retograde \music-expression
> is pretty clear.
Example deleted

>
http://codereview.appspot.com/4126042/diff/3009/input/regression/modal-transf...
> input/regression/modal-transforms.ly:9: ges' aes' a' b'
> heh, I thought that after so many people reviewing it, there'd be
> nothing "substantial" left to comment on.
>
> missing duration:
>   ges'4
> :)
>
>
> oh wait, this is a regtest, not the docs.  ok, never mind, we're 
> not
> fussy about input code syntax here.
And it's a scale, so durations are superfluous.  I deleted
the first duration too.

> http://codereview.appspot.com/4126042/

For continuation see http://codereview.appspot.com/4079064/

Trevor 

[michael.f.ellis, 2011-02-05 16:23:55]

Thanks. This is good feedback.  I agree that an extra argument could
be useful.  I think of this operation as transposition followed by
inversion,  so Pal's example could be implemented this way:

%% ------------------------------------------------
modalTransposedInversion =
#(define-music-function (p l from to-pivot scale music)
   (ly:music? ly:music? ly:music? ly:music? )
     #{
        \modalInversion $to-pivot $scale \modalTranspose $from
$to-pivot $scale $music
     #})

scale = {c g}
original = { c4. c'8 g'4. g8 }
desired =  { g4. g,8 c,4. c8 }
{
    \clef bass
    \original
    \desired
    \modalTransposedInversion c g \scale \original

}
%% ------------------------------------------------

So it could be left to the user, but I imagine that transposition will
be wanted more often than not,  so I have no problem with altering the
definitions accordingly.

In practice, it will be more efficient to code it as Pal suggests,
with two index operations -- although I have no idea whether the
efficiency gain would be significant in terms of LilyPond's total
processing overhead.

I do like arranging and naming the arguments as suggested above.  I
think it will simplify documentation and usage if the arguments to
\modalTranspose and \modalInversion (or \modalTransposedInversion if
you prefer)  are as congruent as possible, so that we can have

\modalTranspose   from-pitch   to-pitch  ...
and
\modalInversion     from-pitch    to-pivot  ...

I also find that this corresponds to the way I think about it
musically.  I see the original sequence, then imagine where I want the
first pitch of the inverted sequence to be.

Does anyone have a strong feeling whether we should rename
\modalInversion to \modalTransposedInversion?  Or perhaps leave
\modalInversion as is and offer \modalTransposedInversion as a
separate function?

Daniel, what's the best way for me to submit the changes?  Does
codereview support a way to locally clone the current version.  And
would you rather have a patch or a new copy?

Cheers,
Mike


On Sat, Feb 5, 2011 at 6:28 AM, Trevor Daniels <t.daniels@treda.co.uk> wrote:
>
> Pal wrote Friday, February 04, 2011 5:11 PM
>
>>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
>> scm/modal-transforms.scm:68: (lambda (pivot-pitch pitch)
>> I'm afraid this is not a good interface, as there are inversions where
>> the pivot is between two notes.
>>
>> e.g. with the scale { c g } I may want to invert
>> { c4. c'8 g'4. g8 } to get { g4. g,8 c,4. c8 }.
>
> Good point!
>
>> I see two ways out:
>> 1. have a source-pitch and a target-pitch, like transposition
>> 2. have a single pitch, which is not a pivot, but the image of the
>> starting pitch in the scale definition.
>
> There is a third way, which I would prefer.  An inversion
> can pivot around a note in the scale, as now, or around a
> point between two adjacent notes in the scale, which is
> missing from the current implementation.
>
> I would suggest permitting the pivot to be either one
> note, as now, or two notes, in which case the pivot is
> a point between the two.
>
>>
http://codereview.appspot.com/4126042/diff/3009/scm/modal-transforms.scm#newc...
>> scm/modal-transforms.scm:83: (index pitch scale)))
>> With my suggestion 1. above these three lines could be written as
>>
>> (+ (index target-pitch scale)
>>   (- (index source-pitch scale)
>>      (index pitch scale)))
>
> With my suggestion your example would then be coded:
>
>  \modalInversion { c g } {c g} { c4. c'8 g'4. g8 }
>
> @Mike: do you see any problems with this?  Would you be
> able to code this up?  I can fix the docs.
>
>> http://codereview.appspot.com/4126042/
>
> Trevor
>
>

[benko.pal, 2011-02-05 17:24:59]

> Thanks. This is good feedback.  I agree that an extra argument could
> be useful.  I think of this operation as transposition followed by
> inversion,  so Pal's example could be implemented this way:
>
> %% ------------------------------------------------
> modalTransposedInversion =
> #(define-music-function (p l from to-pivot scale music)
>   (ly:music? ly:music? ly:music? ly:music? )
>     #{
>        \modalInversion $to-pivot $scale \modalTranspose $from
> $to-pivot $scale $music
>     #})
>
> scale = {c g}
> original = { c4. c'8 g'4. g8 }
> desired =  { g4. g,8 c,4. c8 }
> {
>    \clef bass
>    \original
>    \desired
>    \modalTransposedInversion c g \scale \original
>
> }
> %% ------------------------------------------------
>
> So it could be left to the user, but I imagine that transposition will
> be wanted more often than not,  so I have no problem with altering the
> definitions accordingly.

yes; I just thought that practically the same code can
be used to get the final result, so why not.

> In practice, it will be more efficient to code it as Pal suggests,
> with two index operations -- although I have no idea whether the
> efficiency gain would be significant in terms of LilyPond's total
> processing overhead.

by the way efficiency: my automatic reaction is that
validity check should be moved out from the lambda
expression into the user level function (or just one
level below), but I have absolutely no experience.

> I do like arranging and naming the arguments as suggested above.  I
> think it will simplify documentation and usage if the arguments to
> \modalTranspose and \modalInversion (or \modalTransposedInversion if
> you prefer)  are as congruent as possible, so that we can have
>
> \modalTranspose   from-pitch   to-pitch  ...
> and
> \modalInversion     from-pitch    to-pivot  ...
>
> I also find that this corresponds to the way I think about it
> musically.  I see the original sequence, then imagine where I want the
> first pitch of the inverted sequence to be.
>
> Does anyone have a strong feeling whether we should rename
> \modalInversion to \modalTransposedInversion?  Or perhaps leave
> \modalInversion as is and offer \modalTransposedInversion as a
> separate function?

to me inversion is enough, since, as you say, changing
the pivot (whether it's a scale element or not) means
transposition, i.e. transposition is unseparable from
inversion.

p

[t.daniels_treda.co.uk, 2011-02-05 18:45:47]

Michael Ellis wrote Saturday, February 05, 2011 4:23 PM

> In practice, it will be more efficient to code it as Pal suggests,
> with two index operations -- although I have no idea whether the
> efficiency gain would be significant in terms of LilyPond's total
> processing overhead.

I'm not unhappy with that.  And you're right, the symmetry
of the operations is then quite pleasing.

> Daniel, what's the best way for me to submit the changes?  Does
> codereview support a way to locally clone the current version. 
> And
> would you rather have a patch or a new copy?

Trevor, actually :)

You can download the patchset from Rietfeld using the
"download raw patch set" button.  It should apply to
any recent state of git.

A patch against git with this download applied will be
fine, as would a new copy of the changed files.

Trevor

[michael.f.ellis, 2011-02-14 21:46:28]

Hi Trevor,
Took me a couple of days longer to get to this than I planned.  Sorry
for the delay.  Zip of new versions attached.  Believe it addresses
all issues from rietveld.

-- All scheme functions now have docstrings.
-- All ly:warnings have (_i "warning text")
-- All module names begin with "modal-transforms"
-- \modalInversion now has 2 required pitch arguments per our discussions.

I've also made the argument naming consistent throughout the scheme
and music functions.

Transpose is now
\modalTranspose from to scale music

and inversion is now
\modalInversion around to scale music

so that
\displayLilyMusic \modalInversion e' f'  {c d e f g a b} { c'4 e' g' }

yields {a' f' d'},

i.e. modally inverted "around" e' and then modally transposed
so that e' moves "to"  f'.

Notice that modalInversion  has the argument order changed since our
last discussion  where were considering  "from  pivot" because as I
played with the functions I found it slightly confusing to mentally
map the outcome to the arguments.  I think the new names and order are
easier to think about (in English at least).  Hope this seems ok to
everyone.
Again, thanks for your encouragement and help!

Cheers,
Mike


On Sat, Feb 5, 2011 at 1:45 PM, Trevor Daniels <t.daniels@treda.co.uk> wrote:
>
> Michael Ellis wrote Saturday, February 05, 2011 4:23 PM
>
>> In practice, it will be more efficient to code it as Pal suggests,
>> with two index operations -- although I have no idea whether the
>> efficiency gain would be significant in terms of LilyPond's total
>> processing overhead.
>
> I'm not unhappy with that.  And you're right, the symmetry
> of the operations is then quite pleasing.
>
>> Daniel, what's the best way for me to submit the changes?  Does
>> codereview support a way to locally clone the current version. And
>> would you rather have a patch or a new copy?
>
> Trevor, actually :)
>
> You can download the patchset from Rietfeld using the
> "download raw patch set" button.  It should apply to
> any recent state of git.
>
> A patch against git with this download applied will be
> fine, as would a new copy of the changed files.
>
> Trevor
>
>
>