DDMAL / Pixel.js

🖌 A web-based pixel classification and correction platform
MIT License
9 stars 2 forks source link

Warn when user is about to export empty background layer #265

Closed jacobdgm closed 1 year ago

jacobdgm commented 1 year ago

When we worked through the pixel demo last week, several groups forgot to select the entire image as the 'Selected Region' layer before exporting, causing errors when we tried to use the data to train a model in Rodan. This could have been prevented if, when a user tries to export the background layer without having selected anything, there had been a popup warning the user they were about to do this.

jacobdgm commented 1 year ago

I see now that @martha-thomae had the same idea. I'll just copy and paste from the email she sent to everyone in the lab:

Suggestion: Maybe this could be fixed by providing a warning when the user clicks on ‘Submit to Rodan’ letting them know that they have to provide a Select Region layer to submit to Rodan. So something in the lines of: “You need to provide a ‘Select Region’ layer before submitting to Rodan. Please click on the Select Region layer and use the ‘rectangle’ tool to provide the selected regions. You can click on Shift + A to use the full page as your selected region."

Further/related:

Suggestion: It might be useful to add some type of warning when clicking on ‘Submit to Rodan’ if any of the layers are ‘off’, telling the user something like "Are you sure you don’t want to submit X layer? Make sure all the layers you want to submit are ‘on’.”

martha-thomae commented 1 year ago

Thanks for summarizing this, @jacobdgm.

I think I want to edit my first suggestion (the second one is fine).

Regarding the first point, I think we can even improve it. This is the original suggestion:

Something in the lines of: “You need to provide a ‘Select Region’ layer before submitting to Rodan. Please click on the Select Region layer and use the ‘rectangle’ tool to provide the selected regions. You can click on Shift + A to use the full page as your selected region."

I think it would be even better if, when no selected region is provided, to assume that the user meant to submit the whole image as the selected region. So, maybe just give a warning in case the user did not wanted to submit the whole image but just a few regions of it. So, we might instead say: "You did not provide any 'Select Region' layer. You can correct this by clicking on 'Cancel' and then using the 'Select Region' layer with the 'rectangle' tool to delimit the regions you want to submit to Rodan. Otherwise, if you click on 'Continue,' the whole page will be considered as the selected region and submitted to Rodan"

MrMondrian commented 1 year ago

@martha-thomae @jacobdgm @JoyfulGen Is there ever a case where the user would not want to select the entire background layer? I can fix this by adding a warning, or try and change Pixel's behaviour to take care of the selection automatically.

martha-thomae commented 1 year ago

Hi @MrMondrian! Yes, there was a case for that. It was really useful to send to Rodan only a few selected regions on a page to train the Calvo Method (which learned very well with small amounts of information, you didn't need to go and classify the whole page in pixel, just a few regions). However, I heard in the last meeting that they plan to eliminate the Calvo Method and just leave the Paco's Method. I haven't tested how well Paco learns with only a few regions of a page, we usually use it with at least two pages.

I think that when the user "forgets" to provide a 'selected region layer,' Rodan should just ask if they are sure they don't want to provide a selected region layer, otherwise the whole image will be taken as the selected region.

MrMondrian commented 1 year ago

It looks like pixel already warns the user before they export an empty background layer. @jacobdgm When you tried to export with no background layer, did this warning not appear? If so, do you know what I could do to recreate this issue? @martha-thomae have you had this happen to you as well?

Screenshot 2023-05-25 at 10 13 30 AM

martha-thomae commented 1 year ago

Yes, the warning does exist already. I have seen it before. But, for some reason, it didn't seem to work during the training week, because people were actually able to produce an empty background layer and an empty selected region layer. Somehow they seemed to have circumvented the warning and still got to submit empty layers to rodan. Maybe they clicked twice on the 'submit to rodan' button or something? (since the button doesn't give feedback, that might have happened and maybe the double-clicking somehow went through despite the supposed warning).

The ideal thing would be for the warning to tell you that if you don't provide a selected region layer, it will submit the whole image as the selected region (and generate the background layer based on that). Then the user would have to click 'ok' if that is what they want, or 'cancel' if they want to provide their own selected regions to generate the background layer. If for some reason the warning is circumvented again, then the default behaviour would be to take the whole image as the selected region.

MrMondrian commented 1 year ago

Okay I will start by investigating the error and making the changes you described once I figure it out. I'm new to the pixel code base so a simple error like this might take me some time.

martha-thomae commented 1 year ago

No problem, Anthony. Take your time. Figuring out how people got around the warning will be useful. Thank you!

MrMondrian commented 1 year ago

I've addressed the bug here and implemented the requested feature here.

For the bug, I identified an obvious cause and removed it. After testing I was no longer able to recreate the bug.