LinkedHashMap and funtional programming a problem in JavaScript (SwingJS adaptation only)

[BobHanson]

LinkedHashMap and funtional programming a problem in JavaScript (SwingJS adaptation only)

[JonStargaryen]

LinkedHashMap
I decided to use LinkedHashMap because there are some use cases where the order of categories does matter. E.g., reading and writing of a file should result in the original file. I see that that's not relevant for binary files, however mmCIF basically becomes unreadable for humans when atom_site records are shuffled. In Java, a linked map performs roughly as fast as a HashMap.
I think it's a good solution to ignore the linked aspect of the map impl in JavaScript if it doesn't matter for your application and increases performance drastically.

Stream API
And yeah, as everything, the Java Stream API comes at a price. It's a tradeoff between performance and (subjective) readability. Cases like IntStream.of(input).mapToDobule(i->f*i).toArray() from FixedPointCodec are surely overkill and should be plain loops. I'll check for similar occurrences and replace them.

[JonStargaryen]

a6b3a31 replaces a couple of stream-based impls by loops. I don't see any performance implications but I could imagine that the JavaScript code may benefits from it.

[BobHanson]

Thanks. I'll integrate those into my fork.

…

On Mon, Dec 2, 2019 at 5:18 PM Sebastian Bittrich ***@***.***> wrote: a6b3a31 <a6b3a31> replaces a couple of stream-based impls by loops. I don't see any performance implications but I could imagine that the JavaScript code may benefits from it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4?email_source=notifications&email_token=AEHNCW2T3ZZ6KB6MMCTG7NTQWWXW5A5CNFSM4JTILTKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFXXVAY#issuecomment-560953987>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEHNCWZK6L7M3NXXVIB4BZDQWWXW5ANCNFSM4JTILTKA> .

-- Robert M. Hanson Professor of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

[BobHanson]

ps - just saying that I fully expect the pull request to be closed (and not just accepted) once the related issues are dealt with. It just seemed to me the simplest way to package the suggested changes, particularly in relation to generics. I'm also thinking it would make sense to move all those XxxGeneric classes to a generic/ package. The changes elsewhere are minimal -- just a few hooks to allow for the possibility.

…

On Tue, Dec 3, 2019 at 6:38 AM Robert Hanson ***@***.***> wrote: Thanks. I'll integrate those into my fork. On Mon, Dec 2, 2019 at 5:18 PM Sebastian Bittrich < ***@***.***> wrote: > a6b3a31 > <a6b3a31> > replaces a couple of stream-based impls by loops. I don't see any > performance implications but I could imagine that the JavaScript code may > benefits from it. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#4?email_source=notifications&email_token=AEHNCW2T3ZZ6KB6MMCTG7NTQWWXW5A5CNFSM4JTILTKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFXXVAY#issuecomment-560953987>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AEHNCWZK6L7M3NXXVIB4BZDQWWXW5ANCNFSM4JTILTKA> > . > -- Robert M. Hanson Professor of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr If nature does not answer first what we want, it is better to take what answer we get. -- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

-- Robert M. Hanson Professor of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

[BobHanson]

I realize this issue is closed, and I don't think I can open it myself, but I'd like to suggest that it really is still an issue. Using Functional programming with DemoReadGeneric: (ms)

(JavaScript)
153 niter=11 processStream BINARY StreamParser
175 niter=11 processStream BINARY StreamParser
173 niter=11 processStream BINARY StreamParser
162 niter=11 processStream BINARY StreamParser
1840 niter=10 ------PARSE binary (-GC)
1840 niter=10 ------PARSE binary

vs. allowing the developer to get the raw data for atom_site.cartn_[x|y|z] directly using

double[][] aatoms = atomSite.fillFloat(new String[] { "cartn_x", "cartn_y", "cartn_z" });

(JavaScript)
1 niter=11 processStream BINARY StreamParser
1 niter=11 processStream BINARY StreamParser
1 niter=11 processStream BINARY StreamParser
1 niter=11 processStream BINARY StreamParser
92 niter=10 ------PARSE binary (-GC)
92 niter=10 ------PARSE binary

So that's an enormous speed-up - on the order of 20 to 200 fold.

The effect is not just JavaScript. The speed is considerably faster in Java as well:

Using the (currently required) functional methods:

2 niter=11 processStream BINARY StreamParser
1 niter=11 processStream BINARY StreamParser
21 niter=10 ------PARSE binary (-GC)
32 niter=10 ------PARSE binary

vs. allowing delivery of the raw arrays:

0 niter=11 processStream BINARY StreamParser
1 niter=11 processStream BINARY StreamParser
12 niter=10 ------PARSE binary (-GC)
25 niter=10 ------PARSE binary

Roughly a two-fold improvement, depending upon how you count.

(All these tests involve reading 3j9m, requesting columns for atom_site.cartn_x, cartn_y, and cartn_z, and then averaging cartn_x.)

[JonStargaryen]

The main issue is the way how data of a Column can be accessed, correct?

It's possible write a loop and access individual values using Column#get(int) by specifying the row. That should be already faster than a stream-based impl. Also, it's possible to access binary data directly (i.e. get access to the int[]/double[]/String[] that columns wrap). There are FloatColumn#getBinaryData(), IntColumn#getBinaryData(), and StrColumn#getBinaryData(). These functions are not exposed via the Column interface and will crash and burn when called on mmCIF data.

Some like this should be as fast as it gets:
double[] cartn_x = atom_site.getCartnX().getBinaryData();

[BobHanson]

I can't use .getAtomSite() calls, because that's the 4.7 MB required download. I just can't see downloading that much just to open a binaryCIF file.Kind of defeats the purpose. Take a look at what I have on my fork. I did a lot of refactoring, and you will see that all the generated api methods and classes are no longer in my edition. It's way more refactoring than you would ever be interested in, I know, so I'm not proposing creating a pull request for that. I will just leave it there for reference. I isolated all the api-specific code from all the rest. Added a few interfaces and a few helper classes (model.Builder, text.TextWriter, text.TextReader, binary.BinaryEncoder, for example) that can serve both generic and api configurations. I put all my generic classes in org.rcsb.cif.generic and (for my implementation) moved all the original non-generic business -- what I'm calling the "api" classes that were there (11 MB of Jav -- to my unused/ directory, out of the class path. Not building 10,000 methods does make compiling go a lot faster! The java2script transpiler is a bit sluggish, requiring a few minutes to do all of that, but just seconds to process what I have there now (80 java files total). I now have the download for reading binaryCIF files down to just 42 classes from org.rcsb.cif. And they are pretty trim -- a total of 86K. Just a bit smaller! :) The generic business is fully worked out -- just requires some subset of the property files and a few classes. I went ahead and worked out building and writing as well as reading. Very happy to be able to have a CIF writer now. That could come in handy on other CIF-related projects. Thank you for that! Actually, I haven't tested the writers for reading back in. I should probably do that; almost certain to find issues there. It does turn out that the good-old JavaScript object "hash(?) map" is must faster than java.util.HashMap in JavaScript - leave the hashing to the browser -- and it is definitely first-in, first-out, so that is just what we need, and there is no ordering issue. Besides, the category names are stored in arrays. I presume David (and others? do they exist) can read binaryCIF files that have all-lower-case tags. Right? The nice thing about the property files is that it would be simple to extend those to other CIF dictionaries, not that I am actually going to do that any time soon. Could the schema generator be tweaked to produce these? I'm happy that it only took me a few days to get the JavaScript version working, and especially happy that the extensive use of functional programming gave the transpiler a great work-out, and I found a number of subtle mistakes I had made there. Sorry, Sebastian, I had to remove most of that, though, as though it is very elegant, it was slowing the process down considerably. So, well worth the effort in my opinion. Bob

…

On Fri, Dec 6, 2019 at 12:31 PM Sebastian Bittrich ***@***.***> wrote: The main issue is the way how data of a Column can be accessed, correct? It's possible write a loop and access individual values using Column#get(int) by specifying the row. That should be already faster than a stream-based impl. Also, it's possible to access binary data directly (i.e. get access to the int[]/double[]/String[] that columns wrap). There are FloatColumn#getBinaryData(), IntColumn#getBinaryData(), and StrColumn#getBinaryData(). These functions are not exposed via the Column interface and will crash and burn when called on mmCIF data. Some like this should be as fast as it gets: double[] cartn_x = atom_site.getCartnX().getBinaryData(); — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4?email_source=notifications&email_token=AEHNCW5WKGLMLSW747HULGTQXKLAXA5CNFSM4JTILTKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGE6XZA#issuecomment-562686948>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEHNCW7B4GOUKXE3AAYKL2TQXKLAXANCNFSM4JTILTKA> .

-- Robert M. Hanson Professor of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

[JonStargaryen]

I can't use .getAtomSite() calls, because that's the 4.7 MB required download.

Oh, I understand. If you access categories/columns by name, you have to know the type and cast appropriately. But that way of directly obtaining the binary data should work for your use case.
double[] cartn_x = ((FloatColumn) block.getCategory("atom_site").getColumn("Cartn_x")).getBinaryData();

Take a look at what I have on my fork. I did a lot of refactoring, and you will see that all the generated api methods and classes are no longer in my edition. It's way more refactoring than you would ever be interested in, I know, so I'm not proposing creating a pull request for that. I will just leave it there for reference.

Yeah, I skimmed through it and adapted some further changes I liked.

I presume David (and others? do they exist) can read binaryCIF files that have all-lower-case tags. Right?

The old JavaScript implementation is case sensitive and I would assume that that is still true for the Mol* implementation.

The nice thing about the property files is that it would be simple to extend those to other CIF dictionaries, not that I am actually going to do that any time soon. Could the schema generator be tweaked to produce these?

I'll adapt your approach of the property files in the future. It's a nice idea to split them by category to allow to ignore categories that will be never accessed. During that process, I'll also adapt the schema generation to create these files directly from the dictionaries.

Sorry, Sebastian, I had to remove most of that, though, as though it is very elegant, it was slowing the process down considerably.

No hard feelings. I'm happy that the library is useful and performs well.

Cheers, Sebastian