Skip to content
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

Add option to only retry non-tagged files in classify #1172

Merged
merged 13 commits into from
Sep 13, 2024

Conversation

nielstron
Copy link
Contributor

@nielstron nielstron commented Sep 10, 2024

This is picking up #1137 again.

Works locally, no idea how to unit test this :)

@nielstron
Copy link
Contributor Author

This is now working on my server after I hacked it into the local settings.

Signed-off-by: Niels <[email protected]>
@marcelklehr
Copy link
Member

Hey @nielstron

Really cool that you're taking this on! I'll look into this soon

@marcelklehr marcelklehr self-requested a review September 10, 2024 15:05
Copy link
Member

@marcelklehr marcelklehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach works in so far as it does indeed restrict the classify command to files that haven't been tagged with the recognize tag yet. However: The recognize tag is only assigned by the imagenet, movinet and musicnn classifiers, not by the face recognition classifier. So, when e.g. the classify command fails or is stopped during the imagenet classification, the images it has tagged so far will not be put through face recognition at all, because they have the tag from the imagenet classification run.

@nielstron
Copy link
Contributor Author

nielstron commented Sep 11, 2024

Thanks for the clarification! Any suggestion on how to fix this?

@nielstron
Copy link
Contributor Author

I see two options

  1. Determine seperate whether the face classification pipeline has been run for a file and put it into the queue if not
  2. always put the file in the face classification queue (which I fear to be quite expensive)

For 1 it would be good to know if such a tag exists/how to obtain it.

@marcelklehr
Copy link
Member

Yeah, I agree with your assessment, for 1. you can find if a face detection exists for a file using the following method: https://github.com/nextcloud/recognize/blob/main/lib/Db/FaceDetectionMapper.php#L82

@nielstron
Copy link
Contributor Author

If face detection for a file did not find a face, is the fact that we processed the file still stored somewhere? Or would this re-run classification on all files that don't contain a face?

@marcelklehr
Copy link
Member

Ah, good question. There's no marker for whether a file has been through face recognition. It might make sense to introduce a new magic tag for this.

@nielstron
Copy link
Contributor Author

Okay. This patch now skips running the no-face-recognition classification for tagged files. I don't think I will have time to contribute more elaborate features in the near future.

Copy link
Member

@marcelklehr marcelklehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this works well enough! Thank you!

@marcelklehr marcelklehr merged commit 53bc672 into nextcloud:main Sep 13, 2024
24 of 30 checks passed
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants