credo-science / credo-webapp

Credo web application
MIT License
2 stars 6 forks source link

a static filter max on to handle championships #23

Closed piotrhomola closed 6 years ago

piotrhomola commented 6 years ago

Hi, it would be great to have more tools to build the Contest view - to handle championships or other competition events. Could we start to enrich the Contest options with adding a static filter by maximum pixel brightness (applicable on the images taken during the contest only), say max >120? If possible and secure, could the filter threshold be placed in the link address, as it was done for time parameters? With this simple (I hope) solution we should be able to filter out background hits and provide the first results of the first championships :) Results of course can change in future, once our understanding of the detections improves. Like in weightlifting, you can become a champion even years after the Olympic Games :)

koziomek commented 6 years ago

I've added additional parameter to contest view. It's called 'max_brightness' and sets a filter for displayed detections (brightness is in range 0.0 to 1.0).

piotrhomola commented 6 years ago

The new view works, although I'm afraid my request was not understood, perhaps I did not express myself clearly enough. I understand from the range of your max_brightness and by looking on the views with its different value that max_brightness = 1.0 is the case where all the pixels are white (255), and indeed this value keeps all the hits on the list, and max_brightness = 0.0 when all are black, and indeed none of the images passes this requirement. Am I right? If yes then this parameter tells about average light level in the picture, and all the hits below this average are kept, while we mostly want to impose our filter on brightest pixel, and keep only the events above the level. Ok, but once we have the average will be also helpful, but not alone, can you please add also the paremater filtering over the brightest pixel, scale 0-255? An maybe please also rename the parameters not to mislead the users, e.g. max_brightness -> av_pixel_brightness, and the new parameter: max_pixel_brightness. Then having these two, average and max, will be scientifically much more reasonable than just one. Thanks!

mpawlik commented 6 years ago

Yes, current "brightness filter" operates only on the average brightness.

Regarding the requested functionality, I think that we are reaching slightly beyond our scope. The idea behind filters is to filter incoming data, based on some criteria and then present it or leave it 'hidden' for further investigation. From our point of view, it would be best to do the investigation offline, not in the webapp itself. While it is easy to introduce new filters and remember why a particular image was filtered out, doing this in form of a dynamic filter with user supplied criteria is a completely different use case. Dynamic filtering requires us to reanalyze all images on demand, which is a bad idea when you think about the volume of data, or add filter specific data to each image. Extending metadata too far is generally not a good idea. There are limits of what can be easily indexed and searched by the database. Additionally, I think that at this point we're learning which filters can be useful, it would be best if we could perform experiments with as little cost as possible i.e. offline.

piotrhomola commented 6 years ago

Sorry but it sounds like a misunderstanding. We are now speaking not about a permanet dynamic filtering mechanism, we spekak about CREDO PR, we announced championships and we cannot announce the results because collected data contains artifacts, bad pixels and background. A minimum solution to handle the situation asap could be a simple filter I've proposed 26 days ago, it would most likely help to filter out bad quality hits, if we also have the average brightness filter it would be very much better, but the minimum is max pixel light. So the request does not concern a permanent solution, here I fully agree with your point, and we also discussed the thing during the meeting - mass production investigation only offline. If it helps to resolve your doubts we could have our dynamic contest view enabled only for superusers, then they would quickly find the parameters that would make the championships results sensible, and then we announce the open view with these optimum parameters fixed, not allowing any dynamic manipulation. After this is done, the dynamic view can be closed even for superusers, although it would be of course very nice if a limited number of experienced users could have an opportunity to do a mini-investigation also online, I'm sure it would be helpful for everyone, but it is another story, we can discuss it separately during a meeting, now let us only focus on a solution that would help to handle the championships asap.

mpawlik commented 6 years ago

@piotrhomola I'll stress this again, your feature request translates to significant changes in the data model and database. It's not about the view, we can keep it or remove it, whatever. It's about adding additional columns to database, which are hard to properly remove.

piotrhomola commented 6 years ago

Ok, it looks loke I misunderstood you, we better meet and talk. I thought that if you can filter on average brightness without adding new columns then you could also do filtering on brightest pixels without adding new columns, if this is not the case we should focus on doing the championships view offline, then you will not be bothered.

W dniu środa, 20 czerwca 2018 Maciej Pawlik notifications@github.com napisał(a):

@piotrhomola https://github.com/piotrhomola I'll stress this again, your feature request translates to significant changes in the data model and database. It's not about the view, we can keep it or remove it, whatever. It's about adding additional columns to database, which are hard to properly remove.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/credo-science/credo-webapp/issues/23#issuecomment-398712291, or mute the thread https://github.com/notifications/unsubscribe-auth/Ahue6QJxlno8V1wysYTcDOBoPVXxQIFjks5t-i1cgaJpZM4UMSPs .

koziomek commented 6 years ago

The parameters are now named "avbrightness_max" and "maxbrightness_min". Example: &avbrightness_max=0.2&maxbrightness_min=120

Keep in mind we are calculating them while serving the request.

koziomek commented 6 years ago

@piotrhomola Do you have any feedback?

piotrhomola commented 6 years ago

I was in hospital with my son for some time, limited activity. In short - perfect & thanks! :) We can close this one, I will have some corresponding issues, but perhaps better separately.

2018-06-25 17:19 GMT+02:00 Krzysztof Oziomek notifications@github.com:

@piotrhomola https://github.com/piotrhomola Do you have any feedback?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/credo-science/credo-webapp/issues/23#issuecomment-399989171, or mute the thread https://github.com/notifications/unsubscribe-auth/Ahue6UpFTNQYa4P3bHierQ1mZPNfyOj8ks5uAP9zgaJpZM4UMSPs .