[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13211878#comment-13211878 ] Neil Roeth commented on MATH-742: - Of course, I'm fine moving the general discussion elsewhere. IIRC, that suggestion was made earlier in this thread by someone. :-) To answer your specific questions: bq. I don't understand why the idiom would break down with the addition of a private field; the only requirement is that a class provides accessors to everything one needs to reconstruct an identical object. I meant private fields with no public accessors, and the requirement you state is precisely the one that would make the class impossible to serialize with the idiom while it would be possible with the other methods. bq. Could you give a concrete example where one would need to serialize a RNG (where it must continue drawing from the same sequence)? Pretty much any RNG should continue using the same sequence rather than restarting from a new seed if the sequences are meant to model the same random element. So, a calculation where you do part of the calculation, do some processing, then do some calculation might be an example. However, evaluating whether this particular case needs to be serialized is missing the point. The point is that there are classes (like this RNG) where the constructor preconditions that the idiom depends upon are nowhere near sufficient to guarantee that an identical object can be created. If you prefer, think of class A with a constructor that takes an argument of class B, then creates from that an internal instance of class C that is required to completely define the state of an instance of A. There is no public accessor for C because that field has no meaning outside of the class itself. The instance of class B used to construct the instance of A is useless after the object is constructed, so it is not saved and therefore is not available at serialization/deserialization time to construct a new instance. The idiom cannot handle that, but the default serialization or explicit writeObject() and readObject() methods can (as long as class C is made Serializable). I would be happy to respond to the other, more general points, but since you asked to suspend further comments on the general issue and others are probably tired of the discussion :-), I'll not do so. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13211388#comment-13211388 ] Neil Roeth commented on MATH-742: - Responding to Luc: bq. I think we can simply say this is not supported. Serialization should not be used for long time storage, and even if it can be used for distributed computation with different versions between client and server, we can simply say we don't support it. Completely reasonable. bq. +1 for the concerns of a library, but there are many cases when a simple implements Serializable is sufficient and does not imply too much work. Yes, agreed. I think this is one of them. Responding to Gilles: bq. There is no encapsulation breaking: The code uses only public accessors in order to extract the objectively meaningful data that are needed to define the concept represented by PolynomialSplineFunction (the knots and the set of coefficients of the polynomial functions that are connected at the knots). Well, yes, but add a private field critical to the state of PolynomialFunction and the idiom breaks down. There would be no breakdown if the class itself were responsible for ensuring that readObject() and writeObject() did the right thing. Here's an example: Suppose you have a random number generator class RngGilles that has a constructor that takes a single long integer as an argument. The constructor precondition returns true for any value of the long integer. With that long integer, it initializes a private table of several hundred long integers and a private marker of its current position in the table. When RngGilles.nextRandom() is called, it calculates the value to be returned from the table and its current place in the table , then updates the table and its current place, then returns the value. It does this a billion times, then needs to be serialized and deserialized in order to continue to use that same random number sequence in another process. The idiom would fail in that case, while the default writeObject() and readObject() would succeed. bq. In some sense, the default serialization breaks the encapsulation because the object is deserialized without undergoing the constructor's precondition checks; while the SerializationProxy actually enforces encapsulation by passing the deserialized to the constructor. I understand what you are saying, but the concept of serialization is that when you are done deserializing, you have an object that represents the exact same state as the object that was serialized, so since it already satisfied any constructor preconditions when it was first created, there is no need to check them again. It is, of course, possible to subvert this for some particular class, e.g., through poorly written explicit writeObject() and readObject() methods, or by failing to mark some fields transient and making methods that use the transient field handle that, but I'd call that a bug in that particular object's serialization implementation, not a general failure of serialization that needs to be handled outside the class by serialization wrappers. ISTM that the idiom basically boils down to deconstructing objects to their primitive fields and then reconstructing them from those primitives, completely bypassing the whole Serialization mechanism except for those primitive fields. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13211077#comment-13211077 ] Neil Roeth commented on MATH-742: - First, thanks for continuing to respond, I do want to understand the reasoning behind this decision and you are helping make it clear. {quote} We are already in trouble, because of this lack of policy, for several classes for which Serializable is a supposedly innocuous feature (e.g. fields that should be transient cannot be because no explicit serialization is implemented). {quote} Do you mean that if you corrected this and made them transient, you would introduce an incompatibility with objects serialized using older versions of the class? If not that, then could you explain? {quote} In the Commons project, there is a commitment that minor releases must be fully compatible; that implies that relying on a default serialization would prevent any change to internal structure of a class. {quote} I understand completely about not making changes in minor releases. Since that commitment is for minor releases, that implies that it could be done at the next major release, right? {quote} Maybe that you are not aware of the constraints imposed by Serializable; maybe that you don't care because in your use-case, you'll never be confronted to the problem of a wrong serialized form. But another user's use-case might bring him here complaining about our inconsistent support of Serializable. Would he be less right than you? {quote} Not aware is more likely than don't care. I may be ignorant but I'm not callous. :-) I understand that suddenly changing how a class is serialized could break things for use cases other than mine and that is no less right than mine. Another reason to restrict the change to a major version. {quote} It is always trivial to add implements Serializable to a class definition. But it is not trivial to do the implementation in the right way; just adding implements Serializable is not the right way, never. Again, it could be good enough for some purpose, but the wish for CM to be an example of good Java programming, is not compatible with statements such as We know it's sloppy, so don't use it whenever you need something that works {quote} This is the core of the issue. I can see that implementing Serializable might require special code in some cases, but why is just adding implements Serializable _never_ the right way? You provided an example of wrapper code to do serialization for a class that is not serializable, but would you provide an example of how to transform a class that is not serializable directly into one that is? I.e., not with a wrapper, but by adding implements Serializable and then adding the explicit serialization methods to the class itself? I would like to see an example of good Java programming with regard to serialization. {quote} The purpose of CM is to provide robust implementations of mathematical utilities; supporting distributed applications is a totally different game. {quote} In my business, we often do massive calculations that require hundreds or thousands of cpu-hours that need to be completed overnight, so they need to be distributed. Regardless of how robust the implementation is otherwise, if it does not support distribution, then that alone might make it unusable. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13211195#comment-13211195 ] Neil Roeth commented on MATH-742: - {quote} It was an example of a case where the appropriate serialization code might be missing. In other words, because it is so simple to add Serializable just in case, we create problems for later. {quote} I am not understanding what problems this could cause - can you give some explicit examples? I don't know what your criteria are for appropriate serialization code - can you describe a case where the default serialization code is not appropriate and _why_ it is not appropriate? As far as I can tell, the writeObject() and readObject() methods in ExceptionContext are unnecessary because they simply do what the default mechanism does anyway (except that it replaces throwing an exception for a non-serializable object with a String that says it is non-serializable). What am I missing? Your code for a class that serializes PolynomialSplineFunction pulls coefficients[] out of the underlying PolynomialFunction class and serializes them. Why do that instead of just using PolynomialFunction.writeObject()? Why is it better programming practice to create separate serialization classes that have deep knowledge of the class structure of each of its data members rather than encapsulate the serialization in each of those classes? If PolynomialFunction implements Serializable, then either PolynomialFunction.writeObject() will do the right thing or it is a bug in PolynomialFunction's implementation of writeObject() - a serializer for PolynomialSplineFunction shouldn't have to take on the responsibility of serializing the guts of PolynomialFunction. I don't see how breaking encapsulation like this is an improvement over the default mechanism, which doesn't break it. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13209596#comment-13209596 ] Neil Roeth commented on MATH-742: - I'm kind of surprised at this result. One developer out of three objected, and that's a consensus? That developer basically said this class should only be Serialized if X, Y and Z were true. I responded that yes, indeed, it was precisely X,Y and Z that were true and in fact were what led to my request. I'd have thought that, all objections having been answered, it would have been a done deal then. Particularly since PolynomialFunction, a class in the same package, is already Serializable. Instead, Won't Fix? What objections were raised that were not answered? Luc, the work arounds you propose are poorer practice than using Serializable exactly as it was intended. The work arounds both boil down to implement serialization of PolynomialSplineFunction through home grown custom code instead of simply adding the Java standard 'implements Serializable'. That is not a better solution, so I am going to keep my locally customized version of the class and hope you someday change your mind. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13210035#comment-13210035 ] Neil Roeth commented on MATH-742: - Gilles, In many cases, the default serialization mechanism is perfectly adequate, as in this case where no one, not even the library maintainers, actually needs to do anything nearly as complex as this code you supplied. Is there any possible problem that adding implements Serializable in this one class could cause? The only possible problem you raised is that someone, somewhere, might possibly save this class as long term storage. How about you make this one class Serializable, and if you get a bug report that it doesn't work when someone uses it for long term storage, you just tell that person Don't do that? Meanwhile, the rest of us can go ahead and use the Serializable class properly, in exactly the circumstances that you stated serialization was for, i.e., passing data around in distributed processing. There is no reason to saddle my simple, isolated issue with the burden of proving that the Commons Math team must commit to making the whole library Serializable. It doesn't have to commit to that. It is perfectly fine to make only part of the library Serializable. The last time I checked, only part of the Java API was Serializable. Just add implements Serializable to this class. Then, put a big, bold statement on the web page that says, We made some parts of Commons Math Serializable where it was trivial to do so, but we do not plan at this time to tackle the parts that are not trivial. Patches to do the latter are welcome, as long as they do it IN THE RIGHT WAY. Then declare success and be happy that you made the library more useful. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13205452#comment-13205452 ] Neil Roeth commented on MATH-742: - Luc, that is precisely my situation. I have a grid computing application where my user-defined objects are returned from the grid, and they contain a PolynomialSplineFunction that is a critical part of the calculated result. Gilles, this is the plumbing needed for distributed applications in my case. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-742) Please make PolynomialSplineFunction Serializable
[ https://issues.apache.org/jira/browse/MATH-742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13205607#comment-13205607 ] Neil Roeth commented on MATH-742: - Gilles, {quote}The issue (and my question) is: Is this field needed in the serialization?{quote} Yes. {quote}If it is needed, then again, is Serializable the correct way?{quote} As you said, [Serializable] was meant to ease some of the plumbing needed for distributed applications, so since my application is distributed and needs this plumbing, the answer is again yes. I raised this issue to add Serializable to this one particular class; if you want to discuss globally adding Serializable to every class, that's a different issue. Could you please open a new issue to discuss that topic? Thanks. Please make PolynomialSplineFunction Serializable - Key: MATH-742 URL: https://issues.apache.org/jira/browse/MATH-742 Project: Commons Math Issue Type: Improvement Affects Versions: 2.2 Reporter: Neil Roeth Priority: Minor Attachments: PolynomialSplineFunction.java PolynomialSplineFunction is not Serializable, while the very similar PolynomialFunction class in the same package is. All that needs to be done is to add the import: {{import java.io.Serializable;}} and change this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction}} to this: {{public class PolynomialSplineFunction implements DifferentiableUnivariateRealFunction, Serializable}} I made exactly that modification to a local copy and it serialized successfully. Before the change, I got serialization errors. Thanks. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira