Mulan-94 / polarvis

Musings on Polarimetric Vis
MIT License
0 stars 0 forks source link

loading representation images #5

Open o-smirnov opened 3 years ago

o-smirnov commented 3 years ago

Things could definitely be made faster. Pinging @ericmandel, he can help!

ericmandel commented 3 years ago

If I understand correctly, this analysis tool allows people to look at the data for one particular data set/paper at a time. So the displayed data can really be tied to the paper, right?

Right now you're being pretty general: you're downloading a 4096 x 4096 by 4bye = 64Mb image, then doing zoom to fit, which zooms by 0.09. But that 64 Mb can take time to download: 128 seconds on my DSL connection. And after that wait, the displayed image, after zoom to fit, is basically 400 x 400.

I'd recommend making a new (static) FITS image on your server of just that size and zoom, which would be less than 1Mb. When you download that, possibly with the parentFile property set back to the original image for back-end analysis., you'll see the same display but it will load a lot faster. You might want to play with size and zoom to see what looks best: a 1024 x 1024 image some other zoom to make the features larger might show things better and still have pretty fast download (4Mb of download will be less than 10 sec on my DSL, probably OK).

Unless there is some specific reason you can't just make representation images statically?

A few other quick notes:

You might want to change the regions default strokeWidth from 2 to 1: with that crowded field it might make them stand out better. It's a little hard to tell but worth a try. You can simply change the default strokeWidth before loading the regions:

JS9.Regions.opts.strokeWidth = 1

or call JS9.ChangeRegions:

JS9.ChangeRegions("all", {"strokeWidth":1});

(slower, if you've already loaded the regions).

You don't need to load the js9plugins.js file, it's a test file.

ericmandel commented 3 years ago

For example, I attach a blocked/sectioned version of your 68Mb FITS file, which I created from my now defunct funimage program:

# extract a 4096x4096 image section centered at 2036,2052, and then block by 8 to make it a 512x512 result
funimage CYG-0.75-SLO-I.FITS'[4096@2036,4096@2052,8]' CYG-0.75-SLO-I_blk8.FITS
# gzip will compress by a factor of 2 and JS9 will know what to do when it loads
gzip CYG-0.75-SLO-I_blk8.FITS

You can see that the size is approx 1Mb instead of 68Mb, so it will load a lot faster. You can probably do the same thing with fcopy in the ftools package from HEASARC (or just use funtools, even though its defunct: https://github.com/ericmandel/funtools).

Going this route means you'll have to either change the region coordinates in the regions file or (what might be better, in any case), use WCS coordinates for positions in the regions file ... they must be available, right?

CYG-0.75-SLO-I_blk8.FITS.gz

btw, it looks like you are downloading the whole FITS file, so there is no need to turn on fits2fits in the Preload() call, that just takes more time. Just download directly.

ericmandel commented 3 years ago

Also, re: your regions file, you can do this:

JS9.LoadRegions("foo.reg", {"selectable":false, "removeable":false});

so you don't have to clutter every region with those params.

ericmandel commented 3 years ago

OK, last comment of the evening. If you change "image" to "physical" in cyg_los.reg and use funimage to make a blocked/sectioned image, you can use the same region file, as show below.

The reason is that funimage sets IRAF LTM/LTV keywords in the FITS header which ds9/js9 know how to interpret to deal with shift/blocking. Here are the keywords in the block8 file:

funhead CYG-0.75-SLO-I_blk8.FITS.gz | egrep ^LT
LTV1    =            0.4375000 / IRAF ref. point                                
LTV2    =        6.2500000E-02 / IRAF ref. point                                
LTM1_1  =            0.1250000 / IRAF matrix value                              
LTM1_2  =        0.0000000E+00 / IRAF matrix value                              
LTM2_1  =        0.0000000E+00 / IRAF matrix value                              
LTM2_2  =            0.1250000 / IRAF matrix value  
Screen Shot 2021-02-24 at 6 24 55 PM

I still recommend WCS positions, if you can generate them, of course.

ericmandel commented 3 years ago

Today I will start working on the unacceptably slow performance of pan/zoom when you have thousands of regions loaded. I was being nice and deliberate in updating the regions, which works for our typical X-ray use case (10 regions) but is terrible for you use case. I'll let you know when it's fixed ... thanks for pointing it out ...

Mulan-94 commented 3 years ago

Hi @ericmandel , so sorry for the delay in replying.

but is terrible for you use case. I'll let you know when it's fixed ... thanks for pointing it out .

Yes, and thank you!

You don't need to load the js9plugins.js file, it's a test file.

Noted!

You can simply change the default strokeWidth before loading the regions

Yes, @o-smirnov was of the same opinion, I will change that.

The reason is that funimage sets IRAF LTM/LTV keywords in the FITS header which ds9/js9 know how to interpret to deal with shift/blocking

Awesome! It looked like the image's soul kept leaving it's body.

On the slow loading time, I tried removing the image's history from its header. It seemed to have improved the loading speeds a bit. Now working with the representation files, I suspect that history was the cause of some of the problems

o-smirnov commented 3 years ago

It looked like the image's soul kept leaving it's body.

He he, I was wondering what that reminded me of, but you've put your finger on it. How very Old Testament.

ericmandel commented 3 years ago

Awesome! It looked like the image's soul kept leaving it's body.

Ha, right you are!

As part of the upcoming optimization, I'll also send you details of the alternate low-level format for storing your regions that should make them load much faster initially. Parsing 2000 region strings apparently is slow ... and, in your case, unnecessary if you are generating the regions automatically.

I suspect that history was the cause of some of the problems

You can check the file size before and after removing the header, I suspect you won't see more than 4/68 improvement (since the whole file is 68Mb and the image is 64mb).

Also, when you play with making a representation file, make sure you pass parentFile to point to the original. If I recall, the Bin/Filter plugin should be able to extract a bin 1 section using the original file and display it, in case someone wants to zoom back in on a region for very detailed analysis. In other words, if you supply a bin 8 file to start, there is still the capability of looking at a section with bin 1.

OK ... now I have to change one of the lowest-level region routines ... wish me luck!

Mulan-94 commented 3 years ago

You don't need to load the js9plugins.js file, it's a test file.

@ericmandel when I remove this file, all my menus at the top bar disappear

ericmandel commented 3 years ago

@Mulan-94 My mistake! You do need js9plugins.js, I misread it as plugintest.js! Sorry ... (the url http://cygnus.ratt-ru.org/cygnus/ is no longer available, so I can't check it.)

Mulan-94 commented 3 years ago

Sorry about that, it is there now

ericmandel commented 3 years ago

Hi @Mulan-94 and @o-smirnov,

I tried three different approaches to improve performance when panning via the mouse, and two of them gave pretty decent results:

  1. I profiled the code and fixed an obvious slowdown (dealing with the new group support, in GitHub but not yet in any release, so you might not even have seen this). I made several other minor optimizations and it seems to be faster by 30%, not bad, but not enough by itself.
  2. I tried and failed to implement a faster algorithm for refreshing shapes, based on JS9's pre v3 algorithm which manipulated the underlying fabric.js graphics directly. It was close but no cigar (as some would say) ... too much book-keeping is required to know how the canvas changed.
  3. I added a JS9.globalOpts.panRefreshLimit property (set to 500) which, when panning using the mouse, avoids refreshing the shapes until mouseup if the number of shapes exceed the limit. So when you start panning with > 500 shapes, the regions temporarily disappear, then reappear when you let go of the mouse button. This makes panning work as expected, at a price (not seeing the regions temporarily) that I can live with. But can you?

I pushed a new "regions" branch to GitHub that contains this new support. If you were able to clone (or pull) JS9, switch to that branch, and try things out, you can let me know if this works for you, and we can take it from there.

Aside from the ever-present dangers of optimizing code, I made a number of relatively minor changes to clean things up, and (automatic tests notwithstanding) there is a chance I broke something. I will continue testing here, but please me know if you see anything unexpected.

Mulan-94 commented 3 years ago

This makes panning work as expected, at a price (not seeing the regions temporarily) that I can live with. But can you?

In my opinion, this is ok.

I will continue testing here, but please me know if you see anything unexpected.

Yes, I'll test it out now. Thank you!

o-smirnov commented 3 years ago

This makes panning work as expected, at a price (not seeing the regions temporarily) that I can live with. But can you?

Offhand I'd think yes -- rather have the snappy performance than lethargic regions slowing down the panning. But the proof of the pudding is in the eating -- I'll wait for @Mulan-94 to redeploy, then give it a test drive.

BTW, the VM this is currently running on is also hosting https://adass2021.ac.za. I'm morbidly curious to see whether Cygnus breaks ADASS first, or vice versa. :)

Mulan-94 commented 3 years ago

@ericmandel I get this notification image

Cygnus breaks ADASS first, or vice versa. :)

@osmirnov I'll bet Cygnus will break it first.

Mulan-94 commented 3 years ago

Sorry I forgot the console output: image

ericmandel commented 3 years ago

Did you just replace js9.js and/or js9support.js? That routine should be in the astroemw.js/astroemw.wasm files ...

If you don't want to install everything, https://js9.si.edu/js9/help/webpage.html describes the files that you need to move into your development space. Aside from js9support.js, js9.js, and js9plugins.js, I think you need:

js9worker.js astroem.js astroemw.js astroemw.wasm

They get loaded asynchronously, depending on site configuration.

Mulan-94 commented 3 years ago

I had replaced everything in my dir, except the prefs files. When I return the old js9.js file, the error goes away but the one in the regions branch still gives this error.

o-smirnov commented 3 years ago

So I can see you've got representations going @Mulan-94, because it loads super fast now (liking the triple plots, too!) Can you set the default top end of the scale to, say, 10 please? It's nice to have more detail in the lobes in the initial view.

So the representation image is a trim 3.9Mb, great. What I don't understand (and this is my ignorance speaking now), when/how does JS9 get to loading more detail from the original image? All I have experience with is directly getting regions via the helper, as I do in the "partners" code.

ericmandel commented 3 years ago

but the one in the regions branch still gives this error.

After cloning the js9 repository afresh, here are my js9 and astroem files:

eric% ls -l astroem*js astroem*.wasm js9.js js9.min.js
-rw-r--r--  1 eric  staff  3129105 Mar  2 06:11 astroem.js
-rw-r--r--  1 eric  staff   136728 Mar  2 06:11 astroemw.js
-rwxr-xr-x  1 eric  staff  1534688 Mar  2 06:11 astroemw.wasm
-rw-r--r--  1 eric  staff   896856 Mar  2 06:18 js9.js
-rw-r--r--  1 eric  staff   449255 Mar  2 06:18 js9.min.js

Can you please check your file sizes in after switching to the regions branch to make sure you have the same? If so, perhaps its something simple like clearing your browser cache?

For comparison here are the master branch file sizes. The astroem files should be the same, but not the js9 files:

eric% ls -l astroem*js astroem*.wasm js9.js js9.min.js
-rw-r--r--  1 eric  staff  3129105 Mar  2 06:11 astroem.js
-rw-r--r--  1 eric  staff   136728 Mar  2 06:11 astroemw.js
-rwxr-xr-x  1 eric  staff  1534688 Mar  2 06:11 astroemw.wasm
-rw-r--r--  1 eric  staff   895097 Mar  2 06:24 js9.js
-rw-r--r--  1 eric  staff   448455 Mar  2 06:24 js9.min.js
Mulan-94 commented 3 years ago

@o-smirnov If I understand you correctly, this is what the docs say:

The smaller in-memory representation file will be used when performing browser-based analysis such as WCS reprojection, 3D plots, and imexam. But the representation file knows its parent, and will send the parent filename to server-side analysis tasks. Thus JS9 performs quick-look analysis on the smaller file and more detailed server-side analysis on the original parent file.

I guess the parent file is always referenced. @ericmandel is this correct?

ericmandel commented 3 years ago

Yes, the server-side analysis should be done with the parent file. You can verify this if you look at the js9node.log output, the exec should reference the parent file, not the representation file. (I didn't get an explicit answer to whether the plots are being made on the server, so I'll still assuming).

Mulan-94 commented 3 years ago

@ericmandel all the files are in check. Only the sizes of the js9*.js files have changed between both branches.

Mulan-94 commented 3 years ago

I didn't get an explicit answer to whether the plots are being made on the server, so I'll still assuming

No the plots are not being made at the server yet..

ericmandel commented 3 years ago

Only the sizes of the js9*.js files have changed between both branches.

Caching problem?

No the plots are not being made at the server yet..

If you are making the plots in the browser, it will use the downloaded blocked file, which might not be what you want.

Mulan-94 commented 3 years ago

Caching problem?

Yes, this was it, works now! my mistake and thank you so much.

If you are making the plots in the browser, it will use the downloaded blocked file, which might not be what you want.

So for now, the plots are pre-made and all that is done is to display them, but this will change in the future

o-smirnov commented 3 years ago

So for now, the plots are pre-made and all that is done is to display them, but this will change in the future

Yes, just to be clear -- the plots are associated with regions on the FITS file, but they are not themselves derived from the FITS file. @Mulan-94 precomputes the data needed to render them and stores that server-side, but that's entirely independent of JS9 at this point.

o-smirnov commented 3 years ago

Yes, the server-side analysis should be done with the parent file.

Ah ok, so essentially we've just rebinned the image to 1k x 1k now for display purposes. Well, that's quite good enough for now. Not sure any of the server-side analysis bits are even applicable to our use case (and some of them give errors), so I would be tempted to ask @ericmandel how to hide that menu. Can we customize menus for a "simplified" user experience?

The ImExam stuff is useful though (3D plot especially!) Slightly annoyingly, I get a "No plots available for the selected region" error whenever I use any of this (although the tools e.g. 3D plot actually work just fine). I'm guessing @Mulan-94 this is your message, thrown when the region doesn't have an associated LoS plot? I think in the callback, you need to distinguish somehow the LoS-linked regions you've created, and any user-defined regions.

ericmandel commented 3 years ago

the plots are pre-made and all that is done is to display them

Ah, I see. That makes me more confident that you should be loading as small a rep file as you can. And if/when you start doing analysis on the server side, the parentFile property will ensure that you do the analysis on the original.

@Mulan-94 totally independent question: when you cloned/pulled the new code, did you get warnings about conflicting filenames in the node_modules/ws directory? If so, I think I tripped over a Mac problem with git.

ericmandel commented 3 years ago

Not sure any of the server-side analysis bits are even applicable to our use case

If you remove the js9Analysis-fits.json and js9Analysis-funtools.json from the analysys-plugins directory in your installed location, those tasks will not be shown. I don't quite know how to make that configurable during the build process and I got tired/lazy trying to think about it.

Also, yes, you can easily make your own customized menubar. See:

file:///Users/eric/js9/demos/js9menustyles.html

thrown when the region doesn't have an associated LoS plot? I think in the callback, you need to distinguish somehow the LoS-linked regions you've created, and any user-defined regions.

LOS regions can have a second "los" tag ...

tags: ["7", "los"]
ericmandel commented 3 years ago

btw, if you have specified "selectable" and "removeable" elsewhere, you might want to specify tags in the comments:

circle(950, 1406, 3)  # LOS, 7

This is how DS9 does it, so you get the benefit of being able to check your regions in DS9 too ... if you use that program.

Mulan-94 commented 3 years ago

did you get warnings about conflicting filenames in the node_modules/ws directory?

@ericmandel Nope, I didn't get any warnings

This is your message, thrown when the region doesn't have an associated LoS plot

@o-smirnov Yes, that's me, I'll change it.

ericmandel commented 3 years ago

OK, thanks, I see name conflicts on Linux but not on Mac ... so I bet you're on a Linux box ... definitely a problem with the Mac's case insensitive file system tripping over itself. Yikes.

o-smirnov commented 3 years ago

Mac's case insensitive file system

How anyone ever thought that would be a good idea, boggles the mind...

ericmandel commented 3 years ago

yeah that definitely comes under the "seemed like a good idea at the time" heading ...

ericmandel commented 3 years ago

Ah ok, so essentially we've just rebinned the image to 1k x 1k now for display purposes. Well, that's quite good enough for now.

Yes, and at the risk of belaboring the obvious ... if you generate your region coordinates using an image-independent coord system like wcs or physical (the latter assuming you've rebinned using an LTM/LTV-aware program or have added those keywords yourself), you won't have to regenerate the regions if/when you change the bin value.