-
Notifications
You must be signed in to change notification settings - Fork 43
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
Test suite #12
Comments
I'll work more on it when the functions will be in a more stable state, but you can see a sneak preview for the Landscape level
Class level
|
Nice...thanks for preparing this! 👍 One reasons for some differences are the different units (everything area related). FRAGSTATS uses hectare, whereas we are currently using something like 'map units'. Assuming that a resolution of xy(1,1) means a cell is 1x1 meter, this translates to square meter at the moment. |
I've been fairly absent in this so far... But quick thought on the units. I like how it currently is coded to use map units. Assuming hectares I think is confusing. Presumably users will have put some thought into projection (a boy can dream!) and thus changing to hectares under the hood may not be desired. Also, great work on all this! I am impressed and humbled by your progress. |
I agree with Jeff. We should be using the map units. I can just apply hectar<->meter correction for the testing purposes. |
Thanks for your input both! That's basically the same discussion as Max and I had, but (I have no reason why) we ended up with thinking it would be more appropriate to have the same results as in FRAGSTATS (to be able to reproduce past results) and make the comparison of different maps more intuitive. Any thoughts on that? |
The only requirement for the user is currently that the cell size (resolution) of the raster must be in meter. However, it doesn't matter if a cell is 1 x 1 meter oder 30 x 30 meteres, that is automatically taken into account by all functions. This gives us the advantages that the results are quiet naturally to interpret (e.g. the edge length in meter is a tangible result) and probably easy to compare to other studies (e.g. using FRAGSTATS). I believe in this context requiring the cell size of the input raster in meters is a rather fair assumption. |
Update of the tests. Most of the values are identical or very similar. However, there is a several that do not fit - for example, AREA_CV, AREA_SD, ENN_MN: Landscape levels
Class levels
|
I 've found one issue comparing the results on a patch level - numbering of patches differs between FRAGSTATS and landscapemetrics: Fragstat
landscapemetrics
|
Could we just use |
Sure, we can do that. |
Do you have an educated opinion regarding the tolerance for your test? Btw, is codecov capable of detecting the nested functions in |
Good questions @marcosci , I do not have good answers though..
|
Yeah, I am lacking that, too.
|
|
Thanks for your input 👍 I just uploaded some tests for all patch level metrics we have at the moment (and where we are to some degree sure they are correct ...). For every metric where we have a complete match with FRAGSTATS, I included a test for that. In every other case, it just tests for consistency of the resulting tibble. I will start a new vignette soon where I will collect everything we have on why our metrics differ and why. |
@Nowosad is there a reason why you use the hardcoded values instead of the tibbles? |
@marcosci probably not;) I will improve that later this week. |
😅 fair enough. I would vote for my solution (if we find out we used the wrong fragstats metrics that does mean we don't have to change hardcoded values in 200 files ...). I pushed tests now for every function on every level. If you feel there is a good test missing, just point it out and will help including it. |
@marcosci Now I've improved and clean many of the tests, etc. I also fixed the travis settings alowing for a longer testing time. |
Nice job @Nowosad, thanks a lot! Seems like I got a bit dizzy while copying around ... Right now, we are a bit off with some metrics, while others are equal. For the ones where we are in the range of FRAGSTATS, I think we have good arguments for why that is the case. The ones way off definitely need some work. You implemented the comparison now for every metric - does this mean that your goal is it to have an exact replica of the FRAGSTATS results? Max and I were also discussing this, as this defines how to progress from now on. |
My thinking now - we should investigate the metrics with results different from FRAGSTATS one by one:
|
I started that now and completed it for the patch level. I uncommented the tests where we don´t get the result, but we think we implemented it correctly. ... and will do the same for class and landscape level. |
Great work Marco. I compile a list of all mismatches (there are still two issues on the patch level):
|
Important note: everytime you uncomment the FRAGSTATS<-> landscapemetrics tests, you should add a hardcoded test instead. This could be very important in the future when working on performance fixes. |
Makes sense :) Weird - shape is the correct when I test it? |
Ah, I vaguely remember the issue we had there ... The documentation in the manual as PDF and html differs - I guess you tested before pulling? |
Ok, the updated list:
|
I hardcoded every metric where it made sense, so fixing units and wrong calculations of sd/cv. The ones where I couldn't think of something meaningful to test against and where we already fail at replicating at patch level are listed in the difference vignette. |
I just had a quick glance, but seems like there are quite a few differences between the results for the podlasie_ccilc raster. My first guess was that this is due to the CRS |
Okay, I had a closer look. Some metrics are identical, some are not. It could also be a problem of the rounding issue |
Yes, it could be due to the geographic CRS. This is why I added this dataset to the package - we should assume that users will provide data not only in a projected CRS... |
Appears to be the rounding issue, as everything is fine with augusta for example. |
Are you sure it is not a projection issue? Augusta has a projected CRS (in meters) while Podlasie has a geographic one (in degrees). Can it influence the results? |
Relatively sure. You can't set a CRS in FRAGSTATS, only the number of cells and resolution - and we use the same properties, not including the projection at all. It s just that FRAGSTATS rounds the resolution of podlasie to 0.003 instead of using 0.002777778. |
Ok, makes sense. |
Some of the metrics only make sense if you have a metric CRS. |
Just flying by here. Big +1 on the geographic coord checks. Without thinking much about this... It feels like not many metrics make much sense without being projected. If that is actually the case (and not just my whim) then requiring projected and not geographic coordinates might make sense... |
So, to structure that a bit:
|
The second option is technically possible, but rather not recommended. There is not a perfect metric CRS for all of the cases. |
OK :-) We could also make it in map units, but then we would lose the advantage to compare against studies in the literature that used FRAGSTATS - I think this would be less desirable than the warning? |
So if we throw a warning and just return the values, wouldn't that be somehow using map units ? |
Moved the units discussion in a new thread. Tests are looking good right now ( > 1400 tests in total 😲 ) and I am just going through the ones with low coverage. |
@Nowosad suggested to test the results of our functions against the results from fragstats.
To do so, @mhesselbarth ran each examplary landscape in the package through fragstats:
fragstat_landscapemetrics.zip
Would make sense to bring the fragstats results in a tidy format and then come up with a streamlined approach to test everything.
The text was updated successfully, but these errors were encountered: