person.'is_valid' is never reseted until recreate cluster

[matiasdelellis]

Hi @stalker314314

How to reproduce:

• For any reason a image is invalidated, the file changed or is removed.
• It set the person.is_valid = FALSE for the WHOLE PERSONS in these image.

I still can see this Person in the group view of user settings page. MM.. This is a bug? For a single photo, what change should I stop showing everything?. In this case, i guess this is the right behavior .. I must show it.

However, when you click on any face of it person, it shows the full image, and when you see the persons in it, it does not show the invalid person.

I guess I should show all people too there.. No?

So, When is a person really invalid and should be ignored?

Returning to the report, the bug really is that when the task is executed, if the resulting cluster is the same -In the sense of the faces what it contains are the same-, the cluster is not rebuilt -Which is logical- but it is not validated again.

Then, I execute the task continuously and always reports:

	Found 0 faces without associated persons for user matias and model 1
	Found 1 changed persons for user matias and model 1
	1995 faces found for clustering
	986 persons found after clustering

[matiasdelellis]

Hi @stalker314314
What do you think of this issue? Should I block appstore?

I did not clarify it but in my case to reproduce it:

• Install the map application
• Scan photos: sudo -u apache occ maps:scan-photos
• In the Maps fronted, If you were never in Africa select a photo that appears there.. 😅
• Right click and select 'Delete geographic data'.

This removes exif tags. We detect it as a change and it invalidate the complete person..

[stalker314314]

Regarding "is_valid", think of it more as "stale" (maybe "viciado" in Spanish?:) So, not really "lie" when invalid person is shown to user, but more "not really (yet) accurate". I would not worry about this. Ideally, we should not show images that are isProcessed=false for this person!

Regarding this infinite loop, it is definitively a bug (after every finished analysis, there should be 0 actionable items), and my plan is to try to understand it . However, I don't see this as a blocker (bug or not, nothing that cannot be fixed with code change and new version in app store:)

[matiasdelellis]

Ok ok..

I agree that it will not be a serious problem, about a year of use and it only happened when I installed maps to test. 😅

Let's think about it calmly .. 😉

[stalker314314]

• So, you never got this message in log:

Clusters already exist, estimated there is no need to recreate them

(just to confirm).

• Also, can you confirm that you still have one person in DB that have is_valid to false (even after cluster creation is done)?

If those two are obvious, one thing that comes to my mind is that we never set is_valid on modified cluster to true (when dealing with modified clusters, in this foreach:

facerecognition/lib/Db/PersonMapper.php

Line 187 in 79b05c9

foreach($newClusters as $newPerson=>$newFaces) {

If this looks reasonable to you, I plan to add one UPDATE in there too

[matiasdelellis]

So, you never got this message in log:

Clusters already exist, estimated there is no need to recreate them

Now I try to reproduce it, but I think not.

Also, can you confirm that you still have one person in DB that have is_valid to false (even after cluster creation is done)?

That is exactly the error.. The flag is_valid is never reset.
As no new photo / face is added, there is a 99% chance that the resulting groups will be the same.

Since the group of faces are the same, simply skip it in the following line:

facerecognition/lib/Db/PersonMapper.php

Lines 195 to 197 in 09fb258

	if ($newFaces === $oldFaces) {
	continue;
	}

Therefore, it does not recreate the cluster at any time. My logic is correct? That comparison I understand ignores that it is invalid .. 😕

If those two are obvious, one thing that comes to my mind is that we never set is_valid on modified cluster to true (when dealing with modified clusters, in this foreach:

I don't know .. I guess the above ... 😞

[stalker314314]

PR: #177