ubarsc / python-fmask

A set of command line utilities and Python modules that implement the ‘fmask’ algorithm
https://www.pythonfmask.org
GNU General Public License v3.0
75 stars 21 forks source link

deprecation of pre-stacked sentinel2 processing #22

Closed gillins closed 4 years ago

gillins commented 5 years ago

Original report by Ian Cooke (Bitbucket: ircwaves, GitHub: ircwaves).


First let me say that we are very grateful that you have contributed this fmask implementation to the open source world. It is an excellent package that provides a key component for the python ecosystem for Sentinel-2 data users.

Version 0.5.0 indicates that passing fmask_sentinel2Stacked.py pre-stacked file of reflectance bands (--toa), and the image of angles (--anglesfile) is obsoleted. This use-case worked out very well for our application where we download the individual bands and the metadata XML, instead of the entire SAFE archive.

@vincentschut is mentioned here indicating that VRT-based resampling may result in pulling shifted data from the overviews. We haven't observed any issue thus far, but perhaps a link to an example (or discussion) can be posted? I've grappled with many resampling/registration issues, so I am quite familiar with how subtle they can be.

I've labelled this minor, as there are various options to work around, but I think it might be helpful for general understanding of the reason for this change.

Thanks,

Ian

gillins commented 5 years ago

Original comment by Neil Flood (Bitbucket: neilflood, GitHub: neilflood).


Hi Ian,

Thanks for your thoughts on this, I appreciate the interest and feedback.

I think the main reason for adding the new --safedir option was because most people seem to use it that way, and telling them how to run a whole set of commands seemed more cumbersome than coding up the same thing internally.

I am a great believer in not breaking things which currently work, so I was not planning to remove the old options, and if you have built a workflow around them, I am happy to leave them in. So, don't worry about that. When I get a moment, I will update the doc so it does not call them obsolete.

When we set this package up, we had originally expected that people would use it as a Python module, and write their own main program to stack things exactly as they wished, and we only added the command line script as a simple way to get started. We were surprised when people just used that directly as they only way in.

With that in mind, I would suggest that you might benefit from using it that way, i.e. build your workflow on top of the Python module, and write your own command line around that. It sounds like you have done that in some form, but talking to the fmask_sentinel2stacked.py command line. No problem either way, and entirely up to you, that is just how I think of it. For our own work here, we have our own private command line which understands our own file naming conventions and so on, so we do not use the fmask_sentinel2stacked.py main program.

Does all that make sense?

re: Vincent's comments about possible mis-registration of overviews in the supplied jp2 files - I have not observed this either, but I work in a quite different workflow. Mostly I am working with files which have been converted out of the supplied jp2 files, and had their overviews recalculated. I wonder if you are in a similar position by using the individual bands - have not tried that option at all. Anyway, I took him seriously enough that I followed his general pattern when I implemented the new --safedir option.

Neil

gillins commented 5 years ago

Original comment by Ian Cooke (Bitbucket: ircwaves, GitHub: ircwaves).


Thanks, Neil.

All of that makes perfect sense. Using the module programmatically (instead of via subprocess) should be on our list. And, you describe what we have implemented very well. We aren't recomputing the overviews, so I hope that maybe Vincent can chime in with a link or example.

Our app, GIPS, uses the VRT as a layer of indirection. We were going to use a VRT with /vsicurl path/urls in it, but remote reads of JP2s seem problematic. Installations can be configured to pull from ESA scihub, or Google Cloud Storage, and are capable of producing many common remote sensing products from many sources. Documentation is quite stale, so if it ever seems of interest to you in your work, please don't hesitate to contact me directly ( 'I COOKE at ACM . ORG'.lower().replace('at', '@').replace(' ', '')).

Ian

neilflood commented 4 years ago

No further action required.