You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gilles (JIRA)" <ji...@apache.org> on 2012/11/09 16:08:13 UTC
[jira] [Created] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Gilles created MATH-894:
---------------------------
Summary: Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
Key: MATH-894
URL: https://issues.apache.org/jira/browse/MATH-894
Project: Commons Math
Issue Type: Improvement
Affects Versions: 3.0
Reporter: Gilles
Assignee: Gilles
Priority: Minor
Fix For: 3.1
Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495695#comment-13495695 ]
Phil Steitz commented on MATH-894:
----------------------------------
I agree that getInternalValues should be deprecated; but its use in DescriptiveStatistics needs to be replaced. One idea on how to do this, per discussion on the mailing list, is to add
{code}
public double applyStatistic(UnivariateStatistic stat) {
return stat.evaluate(internalArray, startIndex, numElements);
}
{code}
Then in DescriptiveStatistics, apply becomes
{code}
public double apply(UnivariateStatistic stat) {
return eDA.applyStatistic(stat);
}
{code}
+1 to deprecate the pre-enum flags and replace with an enum.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497274#comment-13497274 ]
Phil Steitz commented on MATH-894:
----------------------------------
I had the same thought about the dependency and agree it is better not to have it. I wonder if there is an even better design somehow. This is one place where the lack of array reference semantics limits us in Java. I am OK with the subclass solution if we can't think of anything better.
+1 for deprecating the initialCapacity field and setter (but need to leave them in for compatibility until 4.0)
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-894:
------------------------
Description:
Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
See discussion about this change in MATH-757.
Miscellaneous code modifications are also proposed in order to improve encapsulation.
was:
Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
See discussion about this change in MATH-757.
Fix Version/s: 4.0
Summary: Cleanup of "ResizableDoubleArray" ("o.a.c.m.util") (was: Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util"))
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495283#comment-13495283 ]
Gilles commented on MATH-894:
-----------------------------
"ADDITIVE_MODE" and "MULTIPLICATIVE_MODE" are represented by integers whereas they should be defined in an "enum" type.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497177#comment-13497177 ]
Gilles commented on MATH-894:
-----------------------------
I also find it slightly odd that the "expansionFactor" and "contractionCriteria" are "float"s and not "double"s.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496237#comment-13496237 ]
Phil Steitz commented on MATH-894:
----------------------------------
I am sorry but I just realized (thanks to the unit test :) that there is a downside to this change as it changes the contract of clear. The removed reallocation shrunk the internal array back to initialCapacity in size, as stated in the javadoc. Removing the reallocation leaves the internal array at whatever size it was when clear was invoked. I can imagine usages where after expanding an array to be large, someone might want to free the associated storage and shrink the array back to the initial capacity, which clear would do before this change. Could be no one depends on / cares about this; but if we leave this change in, we should remove the statement about restoring the size from the javadoc.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497575#comment-13497575 ]
Gilles commented on MATH-894:
-----------------------------
Would removing the _private_ "initialCapacity" field break compatibility?
It will break serialization but we agreed that we would not support cross-version serialization.
Similarly, would changing the name of a private field ("contractionCriteria") break compatibility?
Also could I add new "double" fields for the expansion factor and the contraction criterion to replace the current "float" ones, or would it break compatibility?
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494054#comment-13494054 ]
Gilles commented on MATH-894:
-----------------------------
Removed call to "contract()" in revision 1407485.
As part of this issue I'd also add a new method:
{code}
public int getCapacity() {
return internalArray.length;
}
{code}
and deprecate the current "getInternalLength()" (which the doc says is there only for the unit tests' sake). "getCapacity" is reminiscent of the argument passed to one of the "ArrayList" constructors.
I'd also deprecate
{code}
public double[] getInternalValues() {
return internalArray;
}
{code}
because it breaks encapsulation, making it possible to destroy the internal consistency of the object.
Any objection?
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-894:
------------------------
Fix Version/s: (was: 3.1)
Remaining (non backwards-compatible) modifications must be postponed to 4.0.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497594#comment-13497594 ]
Gilles commented on MATH-894:
-----------------------------
bq. Would removing [...] private [...] field break compatibility?
Clirr doesn't seem to complain. So I'll go on with removals and replacements as proposed.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496326#comment-13496326 ]
Gilles commented on MATH-894:
-----------------------------
I think that the "initialCapacity" field and the protected method "setInitialCapacity" have become useless since no other method use them (only "clear()" did, before it was modified).
It's not right anyway to have something called "initialSomething" to be changed after initialization...
Shall I deprecate them?
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497525#comment-13497525 ]
Gilles commented on MATH-894:
-----------------------------
New "protected" methods "getArrayRef()" and "getStartIndex()" added in revision 1409475.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498445#comment-13498445 ]
Gilles commented on MATH-894:
-----------------------------
"initialCapacity" field removed in revision 1410109.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497075#comment-13497075 ]
Luc Maisonobe commented on MATH-894:
------------------------------------
bq. Shall we make the "expansionFactor", "contractionCriteria"[1] and "expansionMode" fields "final" (and hence deprecate the corresponding setters)?
Yes, I think so (and I also agree with the name change).
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495255#comment-13495255 ]
Gilles commented on MATH-894:
-----------------------------
Added "getCapacity" in revision 1408280.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496224#comment-13496224 ]
Gilles commented on MATH-894:
-----------------------------
In revision 1408735, I removed the array allocation in "clear()".
Part of the unit test "testSetElementArbitraryExpansion()" relied on the old behaviour. I factored it out into two new tests "testSetElementArbitraryExpansion1()" and "testSetElementArbitraryExpansion2()" and they pass without other changes.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499301#comment-13499301 ]
Gilles commented on MATH-894:
-----------------------------
Deprecated setter for fields that should be declared as "final" (revision 1410658).
Fields cannot be made "final" until we remove the setters in 4.0 (unless we make those setters no-op).
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496199#comment-13496199 ]
Gilles commented on MATH-894:
-----------------------------
Methods "getInternalVelues" and "start" deprecated in revision 1408723.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497051#comment-13497051 ]
Gilles commented on MATH-894:
-----------------------------
Shall we make the "expansionFactor", "contractionCriteria"[1] and "expansionMode" fields "final" (and hence deprecate the corresponding setters)?
I don't really see the usefulness of being able to fiddle with these values during an object's lifetime. I'd rather add copy constructors that would allow to set those parameters to values different from the copied instance.
[1] Shouldn't this variable be named "contractionCriterion"?
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495734#comment-13495734 ]
Gilles commented on MATH-894:
-----------------------------
That's a neat improvement.
I just don't like the name :). "apply" sounds like it's modifying the object's state. What about "compute(UnivariateStatistic stat)"?
Also, what do you think of my suggestion to not reallocate the array upon calling "clear()"?
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497557#comment-13497557 ]
Gilles commented on MATH-894:
-----------------------------
"DescriptiveStatistics" updated in revision 1409509.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-894:
------------------------
Attachment: MATH-894.patch
Please have a look at the attached patch.
Instead of adding a
{code}
public double compute(UnivariateStatistic s)
{code}
in {{o.a.c.m.util.ResizeableDoubleArray}}, I think that it is better to provide a subclass with the additional functionality. Mainly, this avoids a class in "util" to depend on another package.
If useful beyond "DescriptiveStatistics", the subclass can be made "public".
OK to commit?
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496320#comment-13496320 ]
Gilles commented on MATH-894:
-----------------------------
Revision 1408830: Constructor call chain.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495276#comment-13495276 ]
Gilles commented on MATH-894:
-----------------------------
IMO, there is an unnecessary reallocation when calling the "clear()" method.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497655#comment-13497655 ]
Gilles commented on MATH-894:
-----------------------------
It seems rather far-fetched to imagine that an application would rely on the fact that "clear()" resets the size of the array; this is/was a quite deeply internal design choice.
IIRC, some slightly incompatible functional changes are allowed if they improve the intended usage. Let's say that if someone complains, I'll prepare a 3.1.1 release :).
As for "double" vs "float", it's an oddness since there is no reason to not use "double" for those two fields. I don't think that it will entail any mess; I'll add new constructors (with "double" args) and deprecate the old ones. Unless I'm missing something, users will have nothing to do.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498468#comment-13498468 ]
Gilles commented on MATH-894:
-----------------------------
New {{MathArrays.Function}} interface (as per Phil's proposal on the "dev" ML) added in revision 1410121.
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496267#comment-13496267 ]
Gilles commented on MATH-894:
-----------------------------
New enum type ("ExpansionMode") created in revision 1408797.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498471#comment-13498471 ]
Gilles commented on MATH-894:
-----------------------------
Revision 1410126:
* "UnivariateStatistic" extends "MathArrays.Function"
* "DescriptiveStatistics" updated
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497589#comment-13497589 ]
Phil Steitz commented on MATH-894:
----------------------------------
Changing names or removing private fields does not break compatibility. What *has* to remain is the protected setter that would do nothing. This is a little smelly and really does point to the fact that maybe the change to clear() resulting in the field doing nothing should be pushed to 4.0. As long as you leave the float-valued getters and setters in, adding double versions will not break compat, but will create a mess that I would personally rather see pushed to 4.0 (if ever - it does not bother me that the current fields are floats).
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13495793#comment-13495793 ]
Phil Steitz commented on MATH-894:
----------------------------------
I never liked "apply" as a name. IIRC it goes back to the attempt at functional programming style in the early days of the stat package. The idea is that you "apply" a statistic to a set of data like applying a functor. I am fine with calling it "compute" or even "evaluate" in RDA. I think the change you suggest to clear() should be safe and an efficiency improvement. The only problem would be code that somehow depends on unused storage only at the beginning of the array, which is not an advertised invariant. Unit tests should pick up any problems in the public API.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Spurious method call in
"ResizableDoubleArray" ("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496253#comment-13496253 ]
Gilles commented on MATH-894:
-----------------------------
bq. Removing the reallocation leaves the internal array at whatever size it was when clear was invoked.
As it is now, it's a feature!
Previously, "clear" was, for all purposes, synonymous to creating a new object. And when it comes to a user wanting to free resources, that's what he should do (creating a new object).
I think that keeping the size is more useful since it could spare a series of reallocation/copying which might likely occur when the same array is reused (e.g. in a loop).
I'll update the Javadoc.
> Spurious method call in "ResizableDoubleArray" ("o.a.c.m.util")
> ---------------------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-894) Cleanup of "ResizableDoubleArray"
("o.a.c.m.util")
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498924#comment-13498924 ]
Gilles commented on MATH-894:
-----------------------------
"float" -> "double" (in revision 1410469).
> Cleanup of "ResizableDoubleArray" ("o.a.c.m.util")
> --------------------------------------------------
>
> Key: MATH-894
> URL: https://issues.apache.org/jira/browse/MATH-894
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1, 4.0
>
> Attachments: MATH-894.patch
>
>
> Method "addElement" should not call "contract()", as it will almost always make the storage array shrink on the first call since it will be considered "too big" (for just storing one element).
> See discussion about this change in MATH-757.
> Miscellaneous code modifications are also proposed in order to improve encapsulation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira