solution using Counters and added some unit tests

[Maninder-sd]

This is for issue #7758

[Maninder-sd]

I'm not sure how to add a label. It's the only thing failing

[dschult]

The test failure for Label is fixed when a maintainer assigns a label to the PR (click on the "view details" link of the failing test to see a better message than fits in the title of the test). I've assigned one now so that's why it passes.

Using the phrase "fixes #PR_no" or "closes #PR_no" (and other options) instead of "This is for issue #PR_no" will automatically connect the PR with the issue that it is fixing. When the PR is merged, it triggers the closing of the issue.

Thanks for your first submission!

I think this PR is based on the mistaken assumption in #7786 that we can use >= instead of == in the matching function. I guess we should close that PR. When we switch to using >= for matching, it allows monomorphism instead of isomorphism. But that breaks using that matching function for the isomorphism problem. This is a problem with the API for isomorphism/monomorphism. We can't use the same GraphMatcher class with the same edge_match argument for those two problem types.

The tests here check that this edge_match function works for these examples in both the isomorphism and monomorphism problems. But I believe there must be other examples for which the isomorphism problem will break with this PR. Using >= will allow unequal edges to match -- which we don't want to happen for isomorphisms.

The solution to #7758 is to create your own matching function (as you have done for the monomorphism case you are interested in, where the proposed work-around in #7758 fails). I hope to refactor the isomorphism API to make it more obvious that changing from isomorphism to monomorphism will require changing the matching functions.

Do you agree that this proposed matching function will not work for some cases of the isomorphism problem?

[rossbar]

I agree with @dschult - ultimately I think this is an API problem rather than an implementation problem!

One thing this PR does point out is that the test suite isn't perfect on this front. A simple test for isomorphism that passes on main but fails with this proposed change would be a worthwhile addition to the test suite!

[Maninder-sd]

Hi @dschult and @rossbar. Thanks for the thorough comments.
As far as I understand, subgraph isomorphism/monomorphism does a syntactic and semantic check while iterating over all potential maps. Firstly, the syntactic check makes sure that we check == for the number of edges for isomorphism and >= for monomorphism. Then the semantic check calls the matching function. Even if this function uses >=, this should produce the correct check for isomorphism because the graphs are finite (?). I agree that this could be made more explicit in the semantic check. I would like to continue thinking about this API problem if that's okay (until I find another doable issue).

In this context, does an API mean a method of a python class?

[rossbar]

I agree that this could be made more explicit in the semantic check. I would like to continue thinking about this API problem if that's okay (until I find another doable issue).

Personally, I would recommend finding another issue... I don't think there is a code-change solution to this one. Rather, the idea is that users should be encourage to provide a matching function that fits their application rather than relying on the built-in matching functions. The latter case can very easily run into combinatorial explosion if we try to cover all possible cases in the "builtin" matching functions. Indeed, they're already much more complicated than is likely necessary for most cases! So in other words - if you're looking for an opportunity to make code changes, #7758 is likely not a good starting point!

In this context, does an API mean a method of a python class?

What I mean by API is: how users access this functionality; in this case specifically, this refers to the built-in matching functions. I think most users would be much better off defining their own matching functions rather than relying on the built-in ones. However, it's not possible to simply change everything due to backward compatibility constraints. Therefore this is the type of problem that is best addressed (IMO) with comprehensive, high-level documentation changes to emphasize the "better" patterns and de-emphasize the others - in other words, make suggestions to users about how best to use these functions.

[Maninder-sd]

Thanks for your guidance @rossbar I appreciate it!
I'll try a different issue then. This one can be closed