Reduce the code differences between C and Java versions

[kno10]

This patch reduces some trivial differences between the C and Java versions:

• add some comments missing in the Java version
• align some variable names to match the C version
• import static Math.* to be able to use "exp" instead of "Math.exp"
• import static libsvm.svm_params.* to get the constants such as C_SVC as in C
• use NULL instead of null in m4

this should make it easier in the future to keep the C and Java versions in sync.

[cjlin1]

Thanks. We will check this though it will take some time.

…

On 2019-08-19 07:04, Erich Schubert wrote: This patch reduces some trivial differences between the C and Java versions: * add some comments missing in the Java version * align some variable names to match the C version * import static Math.* to be able to use "exp" instead of "Math.exp" * import static libsvm.svm_params.* to get the constants such as C_SVC as in C * use NULL instead of null in m4 this should make it easier in the future to keep the C and Java versions in sync. ------------------------- YOU CAN VIEW, COMMENT ON, OR MERGE THIS PULL REQUEST ONLINE AT: #148 COMMIT SUMMARY * Improve syncronization of Java source with C source * import static + m4 to reduce the diff of Java to C FILE CHANGES * M java/libsvm/svm.java [1] (278) * M java/libsvm/svm.m4 [2] (346) * M java/svm_predict.java [3] (32) * M java/svm_scale.java [4] (21) PATCH LINKS: * https://github.com/cjlin1/libsvm/pull/148.patch * https://github.com/cjlin1/libsvm/pull/148.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub [5], or mute the thread [6]. [ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#148?email_source=notifications\u0026email_token=ABI3BHWQPFJH4LQPDSALCLDQFKR7DA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HF7ZRXA", "url": "#148?email_source=notifications\u0026email_token=ABI3BHWQPFJH4LQPDSALCLDQFKR7DA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HF7ZRXA", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ] Links: ------ [1] https://github.com/cjlin1/libsvm/pull/148/files#diff-0 [2] https://github.com/cjlin1/libsvm/pull/148/files#diff-1 [3] https://github.com/cjlin1/libsvm/pull/148/files#diff-2 [4] https://github.com/cjlin1/libsvm/pull/148/files#diff-3 [5] #148?email_source=notifications&email_token=ABI3BHWQPFJH4LQPDSALCLDQFKR7DA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HF7ZRXA [6] https://github.com/notifications/unsubscribe-auth/ABI3BHWQOTMG3HKUWNKGHRTQFKR7DANCNFSM4INCBJKQ

[ZYQue]

Hello，
Thank you for your commits. Most of your modification looks nice, but we still have some suggestions and wish you can help to change:

1. svm.java is an automatically generated file, so we have no intention to modify it.
2. For the added comment "invalid parameter" in file svm.m4, we think it will be better to use "Unreachable" (same as another place).
3. In file svm.m4, it's ok to follow java style and keep using "null" and "boolean".

Thanks again.

[kno10]

This version of svm.java was generated from svm.m4; but since it is committed to the repository, I also committed the regenerated version.

I chose to use "NULL" and "bool" macros to minimize the delta between the C and the Java version, as this makes it easier to merge any changes from C to Java, even if of course "null" and "NULL" is a trivial delta, this still means you have to compare fewer lines.

Of course your are free to cherry pick only the changes that you like. I've broken the commits into three parts - (A) sync, (B) use static imports, (C) use NULL and bool macros. Maybe you only want to pick the first two commits, minus the derived files.

[cjlin1]

Hello,

We just released a new version of libsvm, in which your
first commit was incorporated.

We couldn't directly sign off your commit because of
some minor problems (one or two changes in your commit 1
indeed should be in commit 2). But in our commit message we did
clearly indicate your contribution.

We decided not to take the commit 2, in which packages
are imported. The reason why we used Math.abs rather
than imported Math package is to avoid naming conflicts. In C we cannot do
this but in java we can. Thus we used specific names since the
beginning. We also need backward compatibility. The change
may cause some existing java code using libsvm to have problems.
Thus in the end we decided not to take it.

Thank you again for the help and contribution.

[kno10]

Hello,
It is perfectly fine with me to adapt the commit.

Wrt. the second commit:
Imports are local to the current class only (that is why you always have to import all the stuff again), and used only at compile time.
So the import static java.lang.Math.* I proposed should yield exactly the same byte code as the original version: there is no "import" in the bytecode; either is expanded by the compiler to the fully qualified java.lang.Math.abs in the bytecode, and must - by the Java language specification and design - not have an effect on outside classes. Encapsulation is very strong in Java.

I have not verified if the class files produced are exactly the same; but they should, unless I again have some smaller mix-up in splitting the commits.

[cjlin1]

Thanks.. My students and I will investigate this a bit more. We are a bit slow but will keep you informed. Thanks again.

…

On 2019-09-10 13:22, Erich Schubert wrote: Hello, It is perfectly fine with me to adapt the commit. Wrt. the second commit: Imports are local to the current class only (that is why you always have to import all the stuff again), and used only at compile time. So the import static java.lang.Math.* I proposed _should_ yield _exactly_ the same byte code as the original version: there is no "import" in the bytecode; either is expanded by the compiler to the fully qualified java.lang.Math.abs in the bytecode, and must - by the Java language specification and design - not have an effect on outside classes. Encapsulation is very strong in Java. I have not verified if the class files produced are exactly the same; but they should, unless I again have some smaller mix-up in splitting the commits. -- You are receiving this because you commented. Reply to this email directly, view it on GitHub [1], or mute the thread [2]. [ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#148?email_source=notifications\u0026email_token=ABI3BHRN3FNJZ7J7N4J4NJTQI76WRA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6MLZMQ#issuecomment-530103474", "url": "#148?email_source=notifications\u0026email_token=ABI3BHRN3FNJZ7J7N4J4NJTQI76WRA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6MLZMQ#issuecomment-530103474", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ] Links: ------ [1] #148?email_source=notifications&email_token=ABI3BHRN3FNJZ7J7N4J4NJTQI76WRA5CNFSM4INCBJK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6MLZMQ#issuecomment-530103474 [2] https://github.com/notifications/unsubscribe-auth/ABI3BHQNJOMKEIKVFBG63DLQI76WRANCNFSM4INCBJKQ

[cjlin1]

We finally checked this issue again
(with the help of Bill Tran)
From
https://docs.oracle.com/javase/tutorial/java/package/usepkgs.html
they mentioned
"Use static import very sparingly. ... readers of the code won't know which class defines a particular static object."
So in the end we decide to be conservative and explicitly specify
package+function.

Thank you again for the pull request.