Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify the FormatMetaparameters function in train_lm.py #52

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

keli78
Copy link

@keli78 keli78 commented Aug 16, 2016

No description provided.

@keli78
Copy link
Author

keli78 commented Aug 16, 2016

I have tested the code (on swbd in Pocolm and the ted_train_lm.sh in Kaldi) and it worked fine.

round_index = 3
x_original = param
x = round(x_original, round_index)
while(temp == x and round_index <= decimal_length):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temp is not a good variable name.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you understood what I was saying.
The idea is to choose the decimal length independently for each metaparameter, in such a way that

  • we don't get exact ones or zeros
  • we don't get collisions producing identical metaparameters.
    Plus, the code needs to be much clearer. Use variable names more carefully.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Dan,
Ok, I'll change the variables' names.
Actually I think my code can deal with the two problems base on following your suggestion as "by e.g. rounding them to 3 decimals but then if you detect that you're about to repeat a
value or produce an exact 0 or 1, add decimal places until that is no
longer the case". I assumed the collisions only occur in the way that the repeating values occur as a pair of two connective name numbers after rounding. I guess maybe this assumption is wrong.
So you want me to rewrite the code to handle cases like more than two values are same or the same values are not connective? Or I just need to reformat the code (e.g. add helper functions) to make it more clear?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean 'consecutive', not 'connective'.
No, it's not just a question of consecutive data-type weights, non-consecutive ones may not have the same values either.
And the code should be rewritten to be much clearer; variable names like 'temp' and 'temp_original' are not OK and the structure anyway is not right.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there is no need to have special code that sets things to 0.001 or
0.999 if they would be rounded to 0 or 1. Just keep adding more digits
until it's not exactly zero or 1.

On Tue, Aug 16, 2016 at 6:13 PM, Ke Li notifications@github.com wrote:

In scripts/train_lm.py
#52 (comment):

  •    if x == '0.00':
    
  •        x = '{:.3g}'.format(param)
    

- out.append(x)

  •    temp_length = len(str(param))
    
  •    if decimal_length < temp_length:
    
  •        decimal_length = temp_length
    
  • decimal_length -= 2
  • temp_original = metaparameters[0]
  • temp = round(temp_original, 3)
  • for param in metaparameters[1:]:
  •    round_index = 3
    
  •    x_original = param
    
  •    x = round(x_original, round_index)
    
  •    while(temp == x and round_index <= decimal_length):
    

Hi Dan,
Ok, I'll change the variables' names.
Actually I think my code can deal with the two problems base on following
your suggestion as "by e.g. rounding them to 3 decimals but then if you
detect that you're about to repeat a
value or produce an exact 0 or 1, add decimal places until that is no
longer the case". I assumed the collisions only occur in the way that the
repeating values occur as a pair of two connective name numbers after
rounding. I guess maybe this assumption is wrong.
So you want me to rewrite the code to handle cases like more than two
values are same or the same values are not connective? Or I just need to
reformat the code (e.g. add helper functions) to make it more clear?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/3bb42c2f1362ffd85d6a4cf0b1fe1a50ab2fe9e1#r75049323,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADJVu64nmjKEoQAWW4y25Qv59MHlb_kBks5qgmBVgaJpZM4Jl6Bt
.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are two possible issues
First,
input = [0.9999999,0.99999999999,0.999999999999999,0.9999999999999999999]
output: train_lm.py: there is a 1.0. metaparameters should be in range (0, 1).
(where there should be no 1.0)

Second,
input = [0.0000012344,0.00000123456,0.000001234567]
output = "0.000001234,0.000001235,0.000001"
the last thing 0.000001 only has 1 significant digit, which may not be what we desire (we want at least three significant digit)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zhouyang, your code looks plausible, although I don't know how to use the decimal module.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just saw your comments.
As for the first one, I see your point. It seems that when the number is too close to 1, the input to RoundToProperDigit() is 1.0 and this function returns 1.0, which cause a false alarm.
While I think when this indeed happens, we should pay attention to this number since it's too close to 1. But I'm not fully sure.
As for the second point, I think if we choose a certain number as precision, for example 3, it doesn't matter much whether "0.000001" or "0.000001234567", right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not really hard to use that, I think it has at three advantages that I can see:
i) We can manipulate the way of rounding, such like ‘ROUND_DOWN’ or ‘ROUND_UP’
ii) 0.00000123123 will be 0.00000123 if we set the precision to 3 (i.e. this is what you mention as ‘AppendDigitUtilNonZero’)
iii) something like str(0.000001) gives ‘1e-6’, which can be bad. Decimal gives a good str representation of it, like ‘0.000001’

However, there are few issue that Dan mention
i) what kind of collision is permissible? My understanding is collision in "data-type weights” is not OK, otherwise it is fine?
ii) Should we check validity in this function, or pass it to other function to check validity
iii) If we find something suspicious, my understanding is to warn user instead of terminating everything?

If you interested in using this code, I will fix these issues so you may use it if you like.

On Aug 18, 2016, at 10:30 PM, Wang Jian notifications@github.com wrote:

In scripts/train_lm.py:

  •    if x == '0.00':
    
  •        x = '{:.3g}'.format(param)
    

- out.append(x)

  •    temp_length = len(str(param))
    
  •    if decimal_length < temp_length:
    
  •        decimal_length = temp_length
    
  • decimal_length -= 2
  • temp_original = metaparameters[0]
  • temp = round(temp_original, 3)
  • for param in metaparameters[1:]:
  •    round_index = 3
    
  •    x_original = param
    
  •    x = round(x_original, round_index)
    
  •    while(temp == x and round_index <= decimal_length):
    
    Zhouyang, your code looks plausible, although I don't know how to use the decimal module.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i) I think it is little hard to precisely quantify what ‘too close’ means,
ii) In many circumstances, 0.000001 and 0.000001234567 can be pretty similar but imagine 1/0.000001 = 1000000.0 but 1/ 0.000001234567 = 810000.6, and in my opinion, significant digits are important than how many digit beyond decimal point.

On Aug 18, 2016, at 10:32 PM, Ke Li notifications@github.com wrote:

In scripts/train_lm.py:

  •    if x == '0.00':
    
  •        x = '{:.3g}'.format(param)
    

- out.append(x)

  •    temp_length = len(str(param))
    
  •    if decimal_length < temp_length:
    
  •        decimal_length = temp_length
    
  • decimal_length -= 2
  • temp_original = metaparameters[0]
  • temp = round(temp_original, 3)
  • for param in metaparameters[1:]:
  •    round_index = 3
    
  •    x_original = param
    
  •    x = round(x_original, round_index)
    
  •    while(temp == x and round_index <= decimal_length):
    
    Sorry I just saw your comments.
    As for the first one, I see your point. It seems that when the number is too close to 1, the input to RoundToProperDigit() is 1.0 and this function returns 1.0, which cause a false alarm.
    While I think when this indeed happens, we should pay attention to this number since it's too close to 1. But I'm not fully sure.
    As for the second point, I think if we choose a certain number as precision, for example 3, it doesn't matter much whether "0.000001" or "0.000001234567", right?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

# should satisfy D1 > D2 > D3 > D4.
# here we only checked the repeating values but not the order relationship
if x == y:
sys.exit("train_lm.py: {0} and {1} are same. metaparameters can not be exactly same.".format(x,y))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is not an error unless x and y are both metaparameters representing data-type weights.
So this could cause failure when there should be no failure.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, it's "the same", not just "same".

y = out[j]
# we will round to the next digit when we detect repeated values
round_digit = round_index + 1
round_max_digit = max(len(str(metaparameters[i])) - 2, len(str(metaparameters[j])) -2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The '... - 2' here, I guess you are assuming the input must start with '0.', but do you check the range of input parameters? Since the input for FormatMetaparameters() is always from the metaparameter file, which is either from optimize_metaparameters.py or from the --bypass-metaparameter-optimization. so I think the check point should be in ParseMetaparameters() function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I assumed it started '0.' Do you think I should double check the input
parameters' range to exclude wired cases like 1.345 or -0.0123 from the
metaparameter file?
I think as train_lm.py wrote, the FormatMetaparameters() is only called in
the optimization step when we don't provide bypass-metaparameters. And the
output of this function is used as bypass-metaparameters, right? If so, I
think checking those parameters and showing warnings if necessary in
FormatMetaparameters() before they are used as bypass-metaparameters and
then parsed makes sense right?
I think check them in ParseMetaparameters() is ok but can be too late when
the generated bypass-metaparameters are not formatted properly?

On Thu, Aug 18, 2016 at 10:23 PM, Wang Jian notifications@github.com
wrote:

In scripts/train_lm.py
#52 (comment):

  •            sys.exit("train_lm.py: there is a {0}. metaparameters should be in range (0, 1).".format(out[i]))
    
  • check2: check repeating values. if happen, then reround them until they are different

  • if there exist same values originally, exit with a warning (this can be very rare)

  • after this step, there should be no any repeating values in set out

  • for i in range(0, size):
  •    # Note: the first #num_train_sets weights are not needed to be checked (they can be the same)
    
  •    if i < num_train_sets:
    
  •        out_param.append(format(out[i]))
    
  •    else:
    
  •        x = out[i]
    
  •        for j in range(i + 1, size):
    
  •            y = out[j]
    
  •            # we will round to the next digit when we detect repeated values
    
  •            round_digit = round_index + 1
    
  •            round_max_digit = max(len(str(metaparameters[i])) - 2, len(str(metaparameters[j])) -2)
    

The '... - 2' here, I guess you are assuming the input must start with
'0.', but do you check the range of input parameters? Since the input for
FormatMetaparameters() is always from the metaparameter file, which is
either from optimize_metaparameters.py or from the --bypass-metaparameter-
optimization. so I think the check point should be in
ParseMetaparameters() function.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/87ca2779cd82ad774f490c940435f54fe5c991b7#r75419776,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANVxSiiWytgQn6cuInbLHm-i_sLNVWovks5qhRObgaJpZM4Jl6Bt
.

Ke Li
Dept. of Electrical and Computer Engineering
Johns Hopkins University
Email: kli26@jhu.edu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, it may be not necessary to check the input for FormatMetaparameters(). But it would be better if we check the args in ParseMetaparameters(), because people can set --bypass-metaparameter-optimization to any possible value, which is not controlled by us.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I should have thought about it. But I found the script
validate_metaparameters.py has done this work (1) range in (0,1); 2)
relationship of data-type weights etc) . If users provide some wired
numbers, they can be detected by this script. So I guess validating them in
ParseMetaparameters() is redundant. How do you think?

On Thu, Aug 18, 2016 at 11:32 PM, Wang Jian notifications@github.com
wrote:

In scripts/train_lm.py
#52 (comment):

  •            sys.exit("train_lm.py: there is a {0}. metaparameters should be in range (0, 1).".format(out[i]))
    
  • check2: check repeating values. if happen, then reround them until they are different

  • if there exist same values originally, exit with a warning (this can be very rare)

  • after this step, there should be no any repeating values in set out

  • for i in range(0, size):
  •    # Note: the first #num_train_sets weights are not needed to be checked (they can be the same)
    
  •    if i < num_train_sets:
    
  •        out_param.append(format(out[i]))
    
  •    else:
    
  •        x = out[i]
    
  •        for j in range(i + 1, size):
    
  •            y = out[j]
    
  •            # we will round to the next digit when we detect repeated values
    
  •            round_digit = round_index + 1
    
  •            round_max_digit = max(len(str(metaparameters[i])) - 2, len(str(metaparameters[j])) -2)
    

Oh yes, it may be not necessary to check the input for
FormatMetaparameters(). But it would be better if we check the args in
ParseMetaparameters(), because people can set --bypass-metaparameter-
optimization to any possible value, which is not controlled by us.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/87ca2779cd82ad774f490c940435f54fe5c991b7#r75423580,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANVxSoSRq24M1FWdCoOin1YJa5MavXMQks5qhSPRgaJpZM4Jl6Bt
.

Ke Li
Dept. of Electrical and Computer Engineering
Johns Hopkins University
Email: kli26@jhu.edu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can leave the work to validate_metaparameters.py.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic, I believe, still not completely right. For example,
For input: metaparameters = [0.1231231,0.1231232,0.1231233,0.12312322]
output: ‘0.1231231,0.1231232,0.1231233,0.1231232’ (the second and forth one find collision)

Second, I think the reason, why input metaparameters = [0.000001,0.000002,0.000003] gives [0,0,0] is because len(str(0.00000001)) = 1e-8 = 5
So, the max precision will be 5 instead of 9. This is a very tricky thing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks. I'll modify it soon.
Ke

On Fri, Aug 19, 2016 at 1:35 AM, chris920820 notifications@github.com
wrote:

In scripts/train_lm.py
#52 (comment):

  •            sys.exit("train_lm.py: there is a {0}. metaparameters should be in range (0, 1).".format(out[i]))
    
  • check2: check repeating values. if happen, then reround them until they are different

  • if there exist same values originally, exit with a warning (this can be very rare)

  • after this step, there should be no any repeating values in set out

  • for i in range(0, size):
  •    # Note: the first #num_train_sets weights are not needed to be checked (they can be the same)
    
  •    if i < num_train_sets:
    
  •        out_param.append(format(out[i]))
    
  •    else:
    
  •        x = out[i]
    
  •        for j in range(i + 1, size):
    
  •            y = out[j]
    
  •            # we will round to the next digit when we detect repeated values
    
  •            round_digit = round_index + 1
    
  •            round_max_digit = max(len(str(metaparameters[i])) - 2, len(str(metaparameters[j])) -2)
    

The logic, I believe, still not completely right. For example,
For input: metaparameters = [0.1231231,0.1231232,0.1231233,0.12312322]
output: ‘0.1231231,0.1231232,0.1231233,0.1231232’ (the second and forth
one find collision)

Second, I think the reason, why input metaparameters =
[0.000001,0.000002,0.000003] gives [0,0,0] is because len(str(0.00000001))
= 1e-8 = 5
So, the max precision will be 5 instead of 9. This is a very tricky thing


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/87ca2779cd82ad774f490c940435f54fe5c991b7#r75429618,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANVxStGZoUOaGLH3DzfOWUzL1ZK-wBMSks5qhUCmgaJpZM4Jl6Bt
.

Ke Li
Dept. of Electrical and Computer Engineering
Johns Hopkins University
Email: kli26@jhu.edu

# this function checks whether there are repeating values in parameters(excluding
# the first #number numbers, which are not data-type weights) and if found, return
# the index of the first pair repeating values
def FindRepeatingValue(parameters):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the work for excluding the first numbers is not done inside the function. You may need to move that line of comment to the caller.

# this function rerounds repeating values until they are different and return the values after rerounding
# if they are indeed the same (collison), exit with a warning (this case can be very rare)
# this function rerounds repeating values until they are different and returns the values after rerounding.
# if they are indeed the same (collison), it exits with a warning (this case can be very rare)
def AdjustValues(number1, number2, index1, index2, ref_number1, ref_number2):
Copy link
Owner

@danpovey danpovey Aug 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if two values are the same (collision), it would be better to just print them with full precision rather than die.
The reason is that collisions are only disallowed in certain circumstances: for the same order of discounting parameter, we cannot have d1 == d2 or d2 == d3. But otherwise, collisions are quite possible. Making this cause the script to die is a bad idea.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So we only let the script die when there are 1) identical
data-directory weights or 2) identical data-type weights for the same order
or 3) exact 0 or 1? I think any of the three cases indicates something must
be wrong and thus we'd better let the script exit with error when any of
them happens. What do you think?
Ke

On Sun, Aug 28, 2016 at 6:54 PM, Daniel Povey notifications@github.com
wrote:

In scripts/train_lm.py
#52 (comment):

 for i in range(0, len(parameters)):
     for j in range(i + 1, len(parameters)):
         if parameters[i] == parameters[j]:
             return(i, j)
 return (-1, -1)

-# this function rerounds repeating values until they are different and return the values after rerounding
-# if they are indeed the same (collison), exit with a warning (this case can be very rare)
+# this function rerounds repeating values until they are different and returns the values after rerounding.
+# if they are indeed the same (collison), it exits with a warning (this case can be very rare)
def AdjustValues(number1, number2, index1, index2, ref_number1, ref_number2):

I think if two values are the same (collision), it would be better to just
print them with full precision rather than die.
The reason is that collisions are only disallowed in certain
circumstances: for the same order of discounting parameter, we cannot have
d1 == d2 or d2 == d3. But otherwise, collisions are quite possible. Making
this cause the script to die is a bad idea.

And we cannot have two data-directory weights be identical [IIRC]. But
otherwise, collisions


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/732e304fd3750c911ce87076cd76aa546090ed2a..e95b774e86bd4ee833b3430ec32a3a052059e26a#r76540845,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANVxSrRvwF7_vkt7jgPcLer-4Hons9Qyks5qkhGJgaJpZM4Jl6Bt
.

Ke Li
Dept. of Electrical and Computer Engineering
Johns Hopkins University
Email: kli26@jhu.edu

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do that-- I just thought it might be simpler to ignore it at this
stage. Up to you.
Also, it's only necessary to avoid collisions within the groups of
discounting constants for a particular order. But avoiding other
collisions where possible may be harmless as long as the program doesn't
die.

On Sun, Aug 28, 2016 at 8:15 PM, Ke Li notifications@github.com wrote:

In scripts/train_lm.py
#52 (comment):

 for i in range(0, len(parameters)):
     for j in range(i + 1, len(parameters)):
         if parameters[i] == parameters[j]:
             return(i, j)
 return (-1, -1)

-# this function rerounds repeating values until they are different and return the values after rerounding
-# if they are indeed the same (collison), exit with a warning (this case can be very rare)
+# this function rerounds repeating values until they are different and returns the values after rerounding.
+# if they are indeed the same (collison), it exits with a warning (this case can be very rare)
def AdjustValues(number1, number2, index1, index2, ref_number1, ref_number2):

I see. So we only let the script die when there are 1) identical
data-directory weights or 2) identical data-type weights for the same order
or 3) exact 0 or 1? I think any of the three cases indicates something must
be wrong and thus we'd better let the script exit with error when any of
them happens. What do you think? Ke
… <#m_-9001043735289973879_>
On Sun, Aug 28, 2016 at 6:54 PM, Daniel Povey _@_.***> wrote: In
scripts/train_lm.py <#52 (comment)
https://github.com/danpovey/pocolm/pull/52#discussion_r76540845>: > for
i in range(0, len(parameters)): > for j in range(i + 1, len(parameters)): >
if parameters[i] == parameters[j]: > return(i, j) > return (-1, -1) > > -#
this function rerounds repeating values until they are different and return
the values after rerounding > -# if they are indeed the same (collison),
exit with a warning (this case can be very rare) > +# this function
rerounds repeating values until they are different and returns the values
after rerounding. > +# if they are indeed the same (collison), it exits
with a warning (this case can be very rare) > def AdjustValues(number1,
number2, index1, index2, ref_number1, ref_number2): I think if two values
are the same (collision), it would be better to just print them with full
precision rather than die. The reason is that collisions are only
disallowed in certain circumstances: for the same order of discounting
parameter, we cannot have d1 == d2 or d2 == d3. But otherwise, collisions
are quite possible. Making this cause the script to die is a bad idea. And
we cannot have two data-directory weights be identical [IIRC]. But
otherwise, collisions — You are receiving this because you authored the
thread. Reply to this email directly, view it on GitHub <
https://github.com/danpovey/pocolm/pull/52/files/
732e304..e95b774
052059e26a#r76540845>, or mute the thread <https://github.com/
notifications/unsubscribe-auth/ANVxSrRvwF7_vkt7jgPcLer-
4Hons9Qyks5qkhGJgaJpZM4Jl6Bt> .
-- Ke Li Dept. of Electrical and Computer Engineering Johns Hopkins
University Email: kli26@jhu.edu


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/732e304fd3750c911ce87076cd76aa546090ed2a..e95b774e86bd4ee833b3430ec32a3a052059e26a#r76542414,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADJVu3MuW1pVXb444aNBWfWKFA-8gfl_ks5qkiSvgaJpZM4Jl6Bt
.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on making this stage simpler. I'll just print out the three kind of
true collisions I mentioned in last email. Even any of the three collisions
indeed happens, it can be found by the validation script later.

On Sun, Aug 28, 2016 at 8:22 PM, Daniel Povey notifications@github.com
wrote:

In scripts/train_lm.py
#52 (comment):

 for i in range(0, len(parameters)):
     for j in range(i + 1, len(parameters)):
         if parameters[i] == parameters[j]:
             return(i, j)
 return (-1, -1)

-# this function rerounds repeating values until they are different and return the values after rerounding
-# if they are indeed the same (collison), exit with a warning (this case can be very rare)
+# this function rerounds repeating values until they are different and returns the values after rerounding.
+# if they are indeed the same (collison), it exits with a warning (this case can be very rare)
def AdjustValues(number1, number2, index1, index2, ref_number1, ref_number2):

You could do that-- I just thought it might be simpler to ignore it at
this stage. Up to you. Also, it's only necessary to avoid collisions
within the groups of discounting constants for a particular order. But
avoiding other collisions where possible may be harmless as long as the
program doesn't die.
… <#m_4228511531846515297_>
On Sun, Aug 28, 2016 at 8:15 PM, Ke Li _@.> wrote: In
scripts/train_lm.py <#52 (comment)
https://github.com/danpovey/pocolm/pull/52#discussion_r76542414>: > for
i in range(0, len(parameters)): > for j in range(i + 1, len(parameters)): >
if parameters[i] == parameters[j]: > return(i, j) > return (-1, -1) > > -#
this function rerounds repeating values until they are different and return
the values after rerounding > -# if they are indeed the same (collison),
exit with a warning (this case can be very rare) > +# this function
rerounds repeating values until they are different and returns the values
after rerounding. > +# if they are indeed the same (collison), it exits
with a warning (this case can be very rare) > def AdjustValues(number1,
number2, index1, index2, ref_number1, ref_number2): I see. So we only let
the script die when there are 1) identical data-directory weights or 2)
identical data-type weights for the same order or 3) exact 0 or 1? I think
any of the three cases indicates something must be wrong and thus we'd
better let the script exit with error when any of them happens. What do you
think? Ke … <#m_-9001043735289973879_> On Sun, Aug 28, 2016 at 6:54 PM,
Daniel Povey *__@_
.> wrote: In scripts/train_lm.py <#52
#52 (comment) <#52 (comment)
https://github.com/danpovey/pocolm/pull/52#discussion_r76540845>>: >
for i in range(0, len(parameters)): > for j in range(i + 1,
len(parameters)): > if parameters[i] == parameters[j]: > return(i, j) >
return (-1, -1) > > -# this function rerounds repeating values until they
are different and return the values after rerounding > -# if they are
indeed the same (collison), exit with a warning (this case can be very
rare) > +# this function rerounds repeating values until they are different
and returns the values after rerounding. > +# if they are indeed the same
(collison), it exits with a warning (this case can be very rare) > def
AdjustValues(number1, number2, index1, index2, ref_number1, ref_number2): I
think if two values are the same (collision), it would be better to just
print them with full precision rather than die. The reason is that
collisions are only disallowed in certain circumstances: for the same order
of discounting parameter, we cannot have d1 == d2 or d2 == d3. But
otherwise, collisions are quite possible. Making this cause the script to
die is a bad idea. And we cannot have two data-directory weights be
identical [IIRC]. But otherwise, collisions — You are receiving this
because you authored the thread. Reply to this email directly, view it on
GitHub < https://github.com/danpovey/pocolm/pull/52/files/ 732e304
732e304
..e95b774
e95b774
052059e26a#r76540845>, or mute the thread <https://github.com/
notifications/unsubscribe-auth/ANVxSrRvwF7_vkt7jgPcLer-
4Hons9Qyks5qkhGJgaJpZM4Jl6Bt> . -- Ke Li Dept. of Electrical and Computer
Engineering Johns Hopkins University Email: *__@
.*** — You are receiving
this because you commented. Reply to this email directly, view it on GitHub
<https://github.com/danpovey/pocolm/pull/52/files/
732e304..e95b774
052059e26a#r76542414>, or mute the thread <https://github.com/
notifications/unsubscribe-auth/ADJVu3MuW1pVXb444aNBWfWKFA-
8gfl_ks5qkiSvgaJpZM4Jl6Bt> .


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/danpovey/pocolm/pull/52/files/732e304fd3750c911ce87076cd76aa546090ed2a..e95b774e86bd4ee833b3430ec32a3a052059e26a#r76542565,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANVxSpYFz7GseHRC70OKJAjQkvxbVxn6ks5qkiYwgaJpZM4Jl6Bt
.

Ke Li
Dept. of Electrical and Computer Engineering
Johns Hopkins University
Email: kli26@jhu.edu

# should satisfy D1 > D2 > D3 > D4.
# here we only checked the repeating values but not the order relationship
if x == y:
sys.exit("train_lm.py: the {0}th and {1}th parameters are both {2}. metaparameters can not be exactly the same.".format(index1, index2, y))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could cause a crash for valid metaparameters if they are the same for different orders. The relation D1>D2>D3>D4 is only within a particular order, there can be repeats for different orders. You might need to reorganize the code a bit.

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

Successfully merging this pull request may close these issues.

4 participants