pytroll / pycoast

Python package for adding coastlines and borders on raster images
http://pycoast.readthedocs.org/en/latest/
GNU General Public License v3.0
40 stars 20 forks source link

Cached overlays are pale #95

Closed lobsiger closed 1 year ago

lobsiger commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

djhoese commented 1 year ago

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 :stuck_out_tongue_winking_eye:

lobsiger commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

djhoese commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 https://github.com/pytroll/pycoast/issues/95#issuecomment-1486702515 and here https://github.com/pytroll/pycoast/issues/95#issuecomment-1486715226 . 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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

_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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/pytroll/pycoast/issues/95#issuecomment-1486715226 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 commented 1 year ago

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 commented 1 year ago

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

lobsiger commented 1 year ago

@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 commented 1 year ago

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. :man_shrugging:

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