Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cached overlays are pale #95

Closed
lobsiger opened this issue Mar 27, 2023 · 77 comments · Fixed by #96
Closed

Cached overlays are pale #95

lobsiger opened this issue Mar 27, 2023 · 77 comments · Fixed by #96
Labels

Comments

@lobsiger
Copy link
Contributor

Problem description

Cached overlay outline and fill colors are pale. See this thread:

https://groups.google.com/g/pytroll/c/7VVgsmstLkQ

@djhoese for unknown reason the google list deletes all my posts. I found out that the pycoast stored overlay.png
is already pale. I made a composite with a clean image of Switzerland and the stored overlay file using ImageMagick.
I get exactly the Satpy result for cached overlays. You can see with the bare eye that the cached overlay.png is pale.

Switzerland-overlay

@djhoese
Copy link
Member

djhoese commented Mar 27, 2023

@lobsiger It looks like google groups decided to require approval of your messages...but then also didn't notice any of the owners of the list that they need approval. Would you like me to approve one of the messages? If so, which time?

@djhoese djhoese added the bug label Mar 27, 2023
@lobsiger
Copy link
Contributor Author

@djhoese what do you mean with "which time"? Do I have to post a message to the google group at some known time? Or can you just approve my first post of this overlay problems thread?

@djhoese
Copy link
Member

djhoese commented Mar 27, 2023

Google groups shows multiple messages from you with different timestamps:

image

I assumed you tried to resend the message multiple times.

@lobsiger
Copy link
Contributor Author

@djhoese I logged in and could post and see my posts. But after logout the posts were deleted. I cleaned my gmail account, changed PW and approved my other e-mail account. Maybe this or some of your help gave me access to google group posts again. Regarding a fix for the pale cache file generation I will be of no help as this code is well over my head :-(. All I found out above is that the saved/cached overlay file is pale already. So the problem seems to be in the generation of this overlay file and not in the later application step.

@mraspaud
Copy link
Member

Hi @lobsiger,

This was a long time ago, but I was the one writing this code at this time.
From what I remember, the problem with the cache is that it can make no assumption on what color will be under it's coastlines, so it becomes lighter naturally. However, now that I think about it, maybe the way to apply the cached coastlines could be improved eg by multiplying instead of pasting the cache?
As you can see here /~https://github.com/pytroll/pycoast/blob/main/pycoast/cw_base.py#L1215
we use the paste function to put the cache lines on top of the background. Maybe there are better methods for this? for example the documentation of paste seems to hint at using something called alpha_composite instead, maybe that's better?
https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.paste

@lobsiger
Copy link
Contributor Author

@mraspaud TBH I do not really understand why the overlay must know (??"can make make no assumptions"??) on top of what kind of background it will be applied later. If I write e.g. borders with 'outline': (255, 0, 0) and 'outline_opacity': 255 I would expect a cached *.png that is fully transparent except for fully opaque red border lines.

Anyway, maybe Image Magick gives a hint here? To apply file foreground.png on top of file background.png the command is:

convert background.png foreground.png -composite result.png

I use this e.g. to overlay MSLP charts on top of GEO satellite images. These charts have no clue what will be underneath.

ukmos-ww-202303280600

@lobsiger
Copy link
Contributor Author

... sorry, I should have taken the black MSLP chart. If you see all white above or even below (?) click on the invisible image :-).

ukmos-bb-202303280600

@mraspaud
Copy link
Member

The thing is that when using antialiased lines as drawn with the AGG backend, some pixels of the lines are somewhat transparent, hence the need to take care of alpha blending.

@lobsiger
Copy link
Contributor Author

@mraspaud O.K. I grasp that writing a fine antialiased line on some background might not be the same thing as writing this line on a transparent canvas (I eliminate antialiasing in my MSLP charts). But what about the fill colors? When these are fully opaque I see no difference. But when I fill e.g. land masses with 'fill': (255,0,0) of 'fill_opacity': 127 I do not understand the difference seen between cached and non cached results below (red in cached image is much more dim).

MetopB-20230327-DAY-0923-natural_color-eurol10-cacheFalse
MetopB-20230327-DAY-0923-natural_color-eurol10-cacheTrue

@lobsiger
Copy link
Contributor Author

@mraspaud @djhoese maybe this is one more hint: If I generate the above image without cache but 'fill' : (127, 0, 0) and 'fill_opacity': 127 I get (apparently) exactly the same red fill colors as with cached 'fill': (255, 0 ,0) and 'fill_opacity': 127.
Is in the cached case the red (255, 0, 0) fill color arithmetically blended with black (0, 0, 0) somehow?

@djhoese
Copy link
Member

djhoese commented Mar 28, 2023

Hhhmmm just a thought, but when the cache image is first created the base Image is created as all black with full transparency (RGBA=(0, 0, 0, 0)). I wonder if we did it (255, 255, 255, 0) if that would make a difference?

@djhoese
Copy link
Member

djhoese commented Mar 28, 2023

Here is where the change would go:

foreground = Image.new("RGBA", (x_size, y_size), (0, 0, 0, 0))

@lobsiger
Copy link
Contributor Author

@djhoese I could try that but I do not expect it to work. If my idea of blending above has some truth then the problem is rather how the lines and fill values are written onto this black surface. The involved (0,0,0,0) pixels should be replaced. For pixels not touched (0,0,0,0) would remain as "no_data" value. Fine lines might suffer the antialiasing problem @mraspaud mentioned.

@djhoese
Copy link
Member

djhoese commented Mar 28, 2023

I won't promise/guarantee that this will fix things, but I've seen something like this in the VisPy library that I maintain. When two colors are blended, especially when transparency is involved, the mix between black (0) and any color is a reduced version of that color, the mix between whitee (255) and any color should at least be the original color. ...this is my hope at least.

@lobsiger
Copy link
Contributor Author

@djhoese O.K. I tested your proposal with 'fill':(255,0,0) and 'fill_opacity': 127 and overlay cache True. Besides different colors I would point to the fact that the minor grid lines now reappear. The antialiasing mentioned by @mraspaud seems to profit from the base RGBA = (255,255,255,0) white canvas. Pinky fill seems like a blend of white and red?

MetopB-20230327-DAY-0923-natural_color-eurol10-cacheTrue-white-base-image

@djhoese
Copy link
Member

djhoese commented Mar 28, 2023

Just to make sure when I test this that I'm doing it the same, could you paste the code you're running to generate the main image part of this (the overlay configuration at least)?

@lobsiger
Copy link
Contributor Author

As a last variant I tested 'fill':(255,0,0) and 'fill_opacity': 127 with overlay cache True starting with a red RBGA = (255, 0, 0, 0) canvas for the overlay file. This basically gives me the original image with no cache ( (red+red)/2=red ??) while the thin minor grid lines now get some red from the @mraspaud mentioned antialiasing. I still think the problem has to do how the colors are applied on the empty base image. No idea whether and where this can be changed/improved.

MetopB-20230327-DAY-0923-natural_color-eurol10-cacheTrue-red-base-image

@lobsiger
Copy link
Contributor Author

@djhoese I'm not sure this is of any help. I make these images with my module LEOstuff.py
that does about everything for LEO sats. I only copied the most relevant overlay parts used.

Area used

eurol10:
description: Euro 10.0km area - Europe
projection:
proj: stere
ellps: WGS84
lat_0: 90.0
lon_0: 0.0
lat_ts: 60.0
shape:
height: 800
width: 1000
area_extent:
lower_left_xy: [-3780000.0, -7644000.0]
upper_right_xy: [3900000.0, -1500000.0]

#############################################################################
# sf is a scale factor depending on image size sf = (width + height) / 3000 #
#############################################################################

sf = (1000 + 800) / 3000

my_coasts  = {'outline': (255, 255,   0), 'outline_opacity': 255, 'width': 1.5*sf, 'level': 1, 'fill': (255, 0, 0), 'fill_opacity': 127}
my_borders = {'outline': (255,   0,   0), 'outline_opacity': 255, 'width': 1.0*sf, 'level': 1}
my_rivers  = {'outline': (  0,   0, 255), 'outline_opacity': 255, 'width': 1.0*sf, 'level': 3}

my_grid  = {'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
            # Opacity 0 will hide the line, values 0 ... 255  EL
            'outline': (255, 255, 255) , 'outline_opacity': 255,
            'minor_outline': (200, 200, 200),'minor_outline_opacity': 127,
            'width': 1.5*sf, 'minor_width': 1.0*sf, 'minor_is_tick': False,
            'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',
            'font': fontdir + '/DejaVuSerif.ttf',
            'fill': 'white', 'fill_opacity': 255, 'font_size': 30*sf}
            # minor_is_tick: False draws a line not ticks, True does ticks
            # label placement l,r,lr for latitude and t,b,tb for longitude

@djhoese
Copy link
Member

djhoese commented Mar 29, 2023

I'm playing around with this in #96 (mostly doc fixes at the moment). But while I was working on making a failing unit test I discovered that if you don't use a background image (so always return the overlays with a transparent background) that the results are always equal. So the cached result, the image generated from reusing the cached image (expected), and generating the image without the cache. This is obvious when you think about it since we never combine the foreground with a background, but I just wanted to point it out.

@djhoese
Copy link
Member

djhoese commented Mar 29, 2023

@lobsiger @mraspaud I've noticed an inconsistency with add_overlays_from_dict. The docstring suggests it should always return a transparent image with the overlays added on to it. However, I've noticed that this is only true if caching is enabled. If you disable caching and provide a background image then it returns the background image with the foreground overlays applied. I think this has to do with the way the code is written and how it treats foreground/background.

Is this OK with us? I think the inconsistency between cache and no cache in this sense makes it undesirable.

@djhoese
Copy link
Member

djhoese commented Mar 29, 2023

hhhmm but maybe that's a performance issue since in the non-cached case you'd be generating a whole new image in memory and then applying it. 🤔

@lobsiger
Copy link
Contributor Author

@djhoese @mraspaud yes of course caching is before all a performance thing especially if you generate many composites of the same scene that are resampled with generate=False (as I do). TBH my English and/or brain is not good enough to understand what you stated 3 post above. Is there a way to cache an overlay without using an image file on a disk? I came to the conclusion that as long as we (must?) use the PIL Image.paste()

https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.paste

there is no way to make final results using cached image files equal to writing all overlays directly on top of the generated satellite image. If I understand it right the Image.paste() uses the A channel as mask. Only if a pixel has A=255 it is not combined with the background. That means the big differences will always be with fine lines (due to antialiasing) and in surfaces filled with some transparency (the lakes I added). That's how I finally noted the difference that must have been there for many years.

@djhoese
Copy link
Member

djhoese commented Mar 29, 2023

@lobsiger I think the antialiasing is going to be an issue for lines of any size, but yes they are maybe more noticeable for fine lines since the percentage of the line that is semi-transparent is much higher.

Some questions for you:

The images you are pasting here in this issue, are they the exact image saved by your code or are they screenshots of what you are seeing? Either way, in your image viewer do you set a background color for images? I have my image viewer set to show a checkerboard pattern underneath an image so when an image is transparent I can see a checkerboard. Here is what I'm seeing with the text I'm making where the square in the upper-right has an opacity of 127. The below are screenshots, so they are what I see in my image viewer.

The final image using caching:

image

The final image without caching:

image

So it seems the blending done by .paste during caching is overwriting the alpha channel instead of adding (taking the maximum is maybe appropriate?) to it. I would expect for a black background with alpha=255 and an overlay shape of alpha=127 that the final result has an overall alpha of 255 but the green color of the square is mixed between black (0, 0, 0) and pure green (0, 255, 0).

Can someone sanity check me on this? I'll look into using some of the other masking and alpha_composite options and see how it changes things.

@lobsiger
Copy link
Contributor Author

@djhoese the images sort from satpy as *.png. Then in a last step I add the legend at left with Image Magick (IM) and change to *.jpg. I do not see a difference of the satpy issued *.png and the *.jpg. Normally I finally make *.webm images because these compress much better than *.jpg. But github does not accept *.webm files added to comments. Except for IM nothing else is used.

@djhoese
Copy link
Member

djhoese commented Mar 29, 2023

Ah JPEG doesn't support transparency anyway. I'm not sure why you wouldn't be seeing a difference between .png and .jpg unless your image viewer is using a black background behind the image. Anyway, I'll keep playing with this. I've got the caching output to be decently consistent and what I expect by using alpha_composite, but without caching it is still doing something funny (not using transparency on the green square so it shows full brightness).

@lobsiger
Copy link
Contributor Author

I think the PIL docs of Image.paste() explain what we see:
"
If a mask is given, this method updates only the regions indicated by the mask. ...... Where the mask is 255, the given image is copied as is. Where the mask is 0, the current value is preserved. Intermediate values will mix the two images together, including their alpha channels if they have them."

As a mask we use the overlay (foreground) A channel. As long as I use colors with opacity 255 (like in the Cities and Points symbols and their labels added) there is no difference. In thin lines even the center pixels might have opacity < 255 and are mixed with the black background "including alpha". That's why borders and rivers are always dim red and dim blue when cached.

@lobsiger
Copy link
Contributor Author

I know that *.jpg has no transparency. Maybe my FireFox browser hides some differences that you see in special image processors. The only problem with transparencies I remember was Himawari *.png images way outside the full disk. I always use nodata_values either black or white. If you think it helps somehow I can further shrink the eurol10 area used so far and post the output *.png without the IM step. As seen above with my all white "Bracknell" MSLP chart it changes when you click on it.

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Here's a very simple test I'm doing:

        from pycoast import ContourWriterAGG

        proj4_string = "+proj=longlat +ellps=WGS84"
        area_extent = (-10.0, -10.0, 10.0, 10.0)
        area_def = FakeAreaDef(proj4_string, area_extent, 200, 200)
        cw = ContourWriterAGG(gshhs_root_dir)

        test_shape_filename2 = tmp_path / "test_shapes2"
        with shapefile.Writer(test_shape_filename2) as test_shapes:
            test_shapes.field('name', 'C')
            test_shapes.poly([[[5.0, 10.0], [10.0, 10.0], [10.0, 5.0], [5.0, 5.0]]])
            test_shapes.record("upper-right-box")

        overlays = {
            "cache": {"file": os.path.join(tmp_path, "pycoast_cache")},
            "shapefiles": [
                {"filename": str(test_shape_filename2), "fill": (0, 255, 0), "outline": (255, 255, 0), "fill_opacity": 127},
            ],
        }

        # Create the original cache file
        print("Before initial cache")
        background_img1 = Image.new("RGBA", (200, 200), (0, 0, 0, 255))
        cached_image1 = cw.add_overlay_from_dict(overlays, area_def, background=background_img1)

        # Reuse the generated cache file
        print("Before cache reuse")
        background_img2 = Image.new("RGBA", (200, 200), (0, 0, 0, 255))
        cached_image2 = cw.add_overlay_from_dict(overlays, area_def, background=background_img2)

        # Create without cache
        overlays.pop("cache")
        print("Before no cache")
        background_img3 = Image.new("RGBA", (200, 200), (0, 0, 0, 255))
        nocache_image = cw.add_overlay_from_dict(overlays, area_def, background=background_img3)

        # Manually (no dict, no cache)
        background_img4 = Image.new("RGBA", (200, 200), (0, 0, 0, 255))
        cw.add_shapefile_shapes(background_img4, area_def, **overlays["shapefiles"][0])

So with changing things in the caching logic to use alpha_composite I get what I expect from these operations in the background_imgX objects. That is, I get a black background and a green square in the upper-right but the green square is "faded" like it has an opacity of 127 applied to it but the square is not transparent like the checkerboard images I showed above.

Now the problem is that in last two cases of the above code (no cache and "manual") the green square is completely bright like the opacity isn't having any effect. I'll have to continue debugging tomorrow.

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

This is just a plain Satpy install. So I'm afraid no tests available.

I'm not sure what you mean. You mean you don't have pytest installed? Otherwise, the changes made in my PR should make sure all test data is now included in the sdist and wheel packages when they are installed. This should mean anyone installing pycoast can run the entire test suite.

I'm a little bit reluctant to pip install into a conda install. Do you think that will work?

It is what I'm doing, but I understand the hesitation. It does mean you have to be careful later on though if you then wanted to install the conda version of the package. One thing you could do is:

pip install --no-deps git+/~https://github.com/djhoese/pycoast.git@bugfix-dull-cache

# do your checks

pip uninstall -y pycoast

If you then do conda list pycoast it should be clear whether or not it has it installed. You can always do conda install --force-reinstall pycoast to make sure you get the conda version fully reinstalled.

Note the --no-deps above which should ensure that pip doesn't install a pip/PyPI version of a package that is already installed via conda (besides pycoast of course).

@lobsiger
Copy link
Contributor Author

O.K. installed with pip. Checked I got your PR file. Tried with generate=True and generate=False. Results are as before.

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Can you try running pytest --pyargs pycoast.tests? You'll have to have pytest installed, but otherwise I think all the dependencies for running the tests should already be installed.

Oof, looks like I get lots of failures on my system, but not with the caching tests 😜

@lobsiger
Copy link
Contributor Author

lobsiger commented Mar 30, 2023

@djhoese I get errors. Everything seems to be there but:

(pytroll) eumetcast@kallisto:~$ pytest --pyargs pycoast.tests
================================================= test session starts ==================================================
platform linux -- Python 3.10.9, pytest-7.2.2, pluggy-1.0.0
rootdir: /home/eumetcast
collected 0 items / 1 error

======================================================== ERRORS ========================================================
_________ ERROR collecting miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py __________
ImportError while importing test module '/home/eumetcast/miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
miniconda3/envs/pytroll/lib/python3.10/importlib/init.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py:32: in
from pytest_lazyfixture import lazy_fixture
E ModuleNotFoundError: No module named 'pytest_lazyfixture'
=============================================== short test summary info ================================================
ERROR miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=================================================== 1 error in 0.87s ===================================================
(pytroll) eumetcast@kallisto:~$

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Ok so I'm currently fixing some things. The tests don't work properly if run from outside a source directory.

Your specific error above is that you're missing the pytest_lazyfixture package:

pip install pytest-lazy-fixture

Sorry about that. I forgot that one.

@lobsiger
Copy link
Contributor Author

lobsiger commented Mar 30, 2023

I get a hell of failed tests. Do I have to start inside the test directory? ... No matter from where I start:

=============== 14 failed, 49 passed, 180 warnings in 56.21s ===================

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Wait until I fix the issues I've discovered related to running tests from an installed package.

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Ok I just pushed a commit. Try reinstalling from the PR and running the tests. You should expect ~6 failing tests due to conda-forge only having Pillow 9.2.0, but the tests being written for Pillow 9.4.0 where some index rounding causes text labels to be shifted by one pixel.

@lobsiger
Copy link
Contributor Author

Yes 6 tests failed:
=============================================== short test summary info ================================================
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::TestContourWriterPIL::test_grid[grid_germ.png-shape3-area_def3-4-grid_kwargs3] - AssertionError: Writing of grid failed
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::TestContourWriterPIL::test_add_grid_from_dict_pil - AssertionError: Writing grid from dict pil failed
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::test_shapes[cw_pil-western_shapes_pil.png-area_def0-specific_kwargs0] - AssertionError: Writing of shapes failed
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::test_shapes[cw_pil-eastern_shapes_pil.png-area_def1-specific_kwargs1] - AssertionError: Writing of shapes failed
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::test_no_scratch[cw_pil-no_h_scratch_pil.png-shape0-area_def0-specific_kwargs0] - AssertionError: Writing of no_scratch failed
FAILED miniconda3/envs/pytroll/lib/python3.10/site-packages/pycoast/tests/test_pycoast.py::test_no_scratch[cw_pil-no_v_scratch_pil.png-shape1-area_def1-specific_kwargs1] - AssertionError: Writing of no_scratch failed
================================ 6 failed, 57 passed, 187 warnings in 69.52s (0:01:09) =================================

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Ok so none of those tests are the cache ones which show/test the things I changed in the PR. So if you run your scripts for generating your images and you're still seeing no change then something else is going on. Or I haven't fixed one of the issues you're running into. You're saying you're not seeing a difference in the PNGs generated by Satpy? How are you running your script? Is there any chance it isn't using the environment you think it is (a hashbang at the top of the file and you're running it like ./my_script.py)?

And by no change, you're saying there is still clearly a difference between the no cache and cached final results?

@lobsiger
Copy link
Contributor Author

I start the script with python Sat-area.py yyyymmddDAY and it gives me a day pass of satellite Sat over area at dateDAY (NIG).
The script uses my module LEOstuff.py in the same directory. I have no other pytroll environment but the one I use on this machine. I just tried with Sentinel-3X over Switzerland today. It looks exactly the way we started. Should I try on another PC?

Sen3X-20230330-DAY-1013-hugo211709n-switzerland-multi-blend_with_weights-cacheFalse
Sen3X-20230330-DAY-1013-hugo211709n-switzerland-multi-blend_with_weights-cacheTrue

@lobsiger
Copy link
Contributor Author

lobsiger commented Mar 30, 2023

The script has on top
#!/usr/bin/env python3
Is this a problem?
But the file is not executable.

Let's call this a day, 23:30 MEZ

@djhoese
Copy link
Member

djhoese commented Mar 30, 2023

Yeah, thanks for sticking with this so late. Normally I would suggest doing pip uninstall -y pycoast then conda uninstall --foce pycoast then do the pip install of my PR again just to be sure, but you ran the tests. The tests that matter passed. The only differences between your above two images are cache on and off?

@lobsiger
Copy link
Contributor Author

lobsiger commented Mar 31, 2023

O.K. I have now one install on another PC where it seems to work. I make a complete new install of miniconda and satpy on the PC from yesterday. Stay tuned ...

@djhoese O.K. I'm back and after installing miniconda3 and everything from scratch and adding your PR it does NOT work on the yesterday PC. The main differences are: On this PC I use my slimmed down scripts that mainly have all configurations to call my module LEOstuff.py that makes about all and finally produces a full list of composites using scn.save_datasets() (NOTE the final s). On the PC where it works I use old versions of my scripts that define everything inside and just do one composite using scn.save_dataset() (NOTE no s at the end).

So I copied one old script on my nonworking PC and this script worked. The only main difference I further found:
In my simple scripts I use "from satpy.scene import Scene" while in my module LEOStuff.py I use "from satpy import Scene, Multiscene, config".

Now I copied the new scripts using modules LEOstuff.py and GEOstuff.py and scn.save_datasets() on the old PC. And these scripts do not work there either while the old simple scripts with scn.save_dataset() still do work as discovered this morning.

So I have now 2 PCs with the same behavior. Simple old scripts work with the PR as expected while the more complicated scripts using my modules show no effect with the PR. Timing the scripts shows that apparently both use the overlay cache and are many seconds faster with cache (once the cache file has been written) compared to without overlay cache.

I'm simply confused. One thing I still noted is that the overlay files have NOT changed with the PR. I had a couple of those images on the production system many days old and copied them aside as overlay_old.png. After the PR I deleted those and they were rewritten. I set those aside as overlay_new.png. A binary file compare using cmp showed them to be identical ?!

@lobsiger
Copy link
Contributor Author

@djhoese forget what I said about save_dataset() versus save_datasets(). In fact my complicated scripts also use save_dataset() in a for loop when lists of composites are done with generate=True. With generate=False (only possible in the complicated scripts) save_objects.append() is used and finally compute_writer_results(save_objects). But neither generate=True nor generate=False works while for the simple scripts generate=True is AFAIK the default because nothing is specified.

I'll now try to slim down working (PR seems good) and non working (PR has no effect) variants of the scripts to a bare minimum.

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 1, 2023

@djhoese @mraspaud after hours of desperation it seems I figured it out. In my sophisticated scripts I save the *.png images using a fill_value e.g. scn.save_dataset(composite, filename, overlay={...}, fill_value=0/255). These fill_values (black/white) are needed for the case of MSLP overlays applied on GEO images by IM. I have white on transparent and black on transparent overlays as shown here #95 (comment) and here #95 (comment) . The point is now that in my older scripts I used no fill_value at all. In fact these fill_values make the #96 fail when using cached pycoast overlays while they have no effect on dim cached overlays in pycoast 1.6.1.

I must slightly adjust the logic in my latest scripts. I rarely use MSLP overlays combined with cached pycoast overlays. So instead of default black I could define these fill values default None. I'm not yet sure what this means for Himawari full disk images though.

@djhoese
Copy link
Member

djhoese commented Apr 1, 2023

MSLP

What does this acronym stand for?

IM

What does this acronym stand for?

When you use fill_value=0 the image should be saved/enhanced/finalized as an RGB. Are you therefore saying that my PR doesn't work with plain RGB images (compared to RGBA images which are generated with the default fill_value=None)?

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 1, 2023

@djhoese

MSLP = Mean Sea Level Pressure (see my referenced images)
IM = Image Magick (best command line image processor)

Below are 3 NOAA20 VIIRS images of Switzerland showing a grid with opacity=127. I made these images with cache enabled and your #96. The pictures were saved as *.png with fill_value=0, fill_value=255 and fill_value=None. Only with fill_value=None I get the same image as when I make them without pycoast overlay cache (where the fill_value seems not to matter for Switzerland at all).

NOAA20-natural_color-switzerland-cacheTrue-fill_value0
NOAA20-natural_color-switzerland-cacheTrue-fill_value255
NOAA20-natural_color-switzerland-cacheTrue-fill_valueNone

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 1, 2023

When you use fill_value=0 the image should be saved/enhanced/finalized as an RGB. Are you therefore saying that my PR doesn't work with plain RGB images (compared to RGBA images which are generated with the default fill_value=None)?

Yes, that's probably what I say. Without cache enabled I get 24Bit (RGB) and 32Bit (RGBA) images that look all the same.
With your PR and cache enabled the 24Bit (RGB) look dim, only the 32Bit (RGBA) with fill_value=None looks as expected.

@djhoese
Copy link
Member

djhoese commented Apr 1, 2023

Ok, I'll try to take a look at this as soon as I can. I should be able to easily add this to my tests and make it work.

At the same time as waiting for your tests, I started working on fixing support for lon/lat projections that don't have a prime meridian at 0 longitude. You can do this in PROJ.4 with +pm=180 for example and I use this for data that crosses the antimeridian. At the same time as all of this I think I've also discovered an additional check at excluding certain polygons from being added that would cause lines across the image. And...I may have a faster way to do this than we do currently (in numpy versus for loops).

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 1, 2023

AFAIK imaging data that crosses the date line is not a problem. In cylindrical projections you can always specify lon_0. One thing to keep in mind is that the GSHHG shapes we are using never cross the date line (there is one sea boundary exception, a BUG I guess). Below today's MSLP overlays of the Pacific. You can also see why I need fill_value=0 here before IM can make the final overlay.

GOES18-20230401-SLO-1200-airmass-opcpe-wc
HIMA9-20230401-SLO-1200-airmass-opcpw-wc

@djhoese
Copy link
Member

djhoese commented Apr 1, 2023

I've experienced problems with non-prime meridian projections. That's why I'm fixing it. For example, the Euro-Asia in the coastline "i" resolution is in view of an image in the pacific ocean but also crosses the regular prime meridian (the antimeridian of my projection).

As for your need for fill value. Could you explain it in more detail? Either way I need to fix RGB handling, but I'm not sure what I should be noticing in your images.

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 1, 2023

Yes Euro-Asia is a problem that we have discussed years back. Can you specify the area in yaml style that makes the problems?
If you are talking about the 'eurasia' and 'euroasis_asia' areas distributed with satpy there should not be problems as those are laea azimuthal projections (though these look ugly to me, see below). If you are talking about cylindric projections that span over more than 180° longitude and include both the 0° and 180° meridian this cannot work with GSHHG and the current pycoast code.

My MSLP overlays are mostly transparent RGBA *.png downloaded from NOAA, DWD and UK MetOffice and heavily preprocessed with IM. If I use white MSLP contours and the area leaves the GEO satellite's data coverage like in the to right corner of the opcpw Himawari image above, I need a black background to see the MSLP contours applied by IM. If my MSLP charts are preprocessed as mainly black as here #95 (comment) I need a white background for MSG3 (now moved to 0°) images that have no data top left.

MSGX-overview-euroasia_10km-selected MSGX-overview-euroasia_asia_10km-selected

@djhoese
Copy link
Member

djhoese commented Apr 2, 2023

So the area is something like this:

my_area:
  description: my_area
  projection:
    proj: longlat
    ellps: WGS84
    pm: 180
    no_defs: null
    type: crs
  shape:
    height: 9359
    width: 7818
  area_extent:
    lower_left_xy: [-2.7155727783203125, -5.613152448272707]
    upper_right_xy: [41.84626430664063, 47.73362392578125]

As for RGB handling, yep I definitely see the errors when I add it to my tests.

@djhoese
Copy link
Member

djhoese commented Apr 2, 2023

@lobsiger I just updated my PR with fixes for RGB. Let me know if it works for you.

@lobsiger
Copy link
Contributor Author

lobsiger commented Apr 3, 2023

@djhoese I have done tests with a NOAA20 pass over Switzerland (see example images with grid overlay above) using the attached script. I have done the same tests with a MetobB pass over the same area. These tests were done on two machines with the latest PR and on one machine without the PR. With the PR all 12 calculated variants optically look the same. Without the PR all cached variants have dim grids. All cached overlays are binary identical. What surprised me is the fact that all 12 image results are binary different. I would have expected that the only difference is between RGB and RGBA images (notably in size).

-rw-r--r-- 1 eum eum 840709 Apr 3 15:21 MetopB-natural_color-switzerland-generateFalse-cacheFalse-fill_value0.png
-rw-r--r-- 1 eum eum 840947 Apr 3 15:22 MetopB-natural_color-switzerland-generateFalse-cacheFalse-fill_value255.png
-rw-r--r-- 1 eum eum 910023 Apr 3 15:22 MetopB-natural_color-switzerland-generateFalse-cacheFalse-fill_valueNone.png
-rw-r--r-- 1 eum eum 842159 Apr 3 15:19 MetopB-natural_color-switzerland-generateFalse-cacheTrue-fill_value0.png
-rw-r--r-- 1 eum eum 842262 Apr 3 15:20 MetopB-natural_color-switzerland-generateFalse-cacheTrue-fill_value255.png
-rw-r--r-- 1 eum eum 911620 Apr 3 15:21 MetopB-natural_color-switzerland-generateFalse-cacheTrue-fill_valueNone.png
-rw-r--r-- 1 eum eum 840709 Apr 3 15:18 MetopB-natural_color-switzerland-generateTrue-cacheFalse-fill_value0.png
-rw-r--r-- 1 eum eum 840947 Apr 3 15:18 MetopB-natural_color-switzerland-generateTrue-cacheFalse-fill_value255.png
-rw-r--r-- 1 eum eum 910023 Apr 3 15:19 MetopB-natural_color-switzerland-generateTrue-cacheFalse-fill_valueNone.png
-rw-r--r-- 1 eum eum 842159 Apr 3 15:16 MetopB-natural_color-switzerland-generateTrue-cacheTrue-fill_value0.png
-rw-r--r-- 1 eum eum 842262 Apr 3 15:17 MetopB-natural_color-switzerland-generateTrue-cacheTrue-fill_value255.png
-rw-r--r-- 1 eum eum 911620 Apr 3 15:17 MetopB-natural_color-switzerland-generateTrue-cacheTrue-fill_valueNone.png

But these binary differences are also present without the PR. So I'm only surprised but not worried. I have also tested on my main system MSLP overlay charts on MSG3 images and the fill_values=0/255 in areas of no satellite data work as expected.

Many thanks for debugging this: Great work, LGTM!

NOAA20test.txt

MSG3-20230403-SLO-1200-airmass-ukmos-ww
MSG3-20230403-SLO-1200-colorized_ir_clouds-ukmos-bb

@djhoese
Copy link
Member

djhoese commented Apr 3, 2023

I'm not sure if you noticed this in your own testing, but in the unit tests I had to allow for a difference between cached and non-cached versions of the overlays of 1 (in the unsigned 8-bit space). It seemed to me that floating point errors were causing the small difference (mainly how the pre-multiplied alpha is used).

With the above floating point differences and PNG internal compression, maybe that's enough to justify the differences you're seeing in file size. 🤷‍♂️

Thanks for all the testing. I'll give the other pycoast maintainers a chance to review the PR and then merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants