Isomorphism: Remove invariant contiguous range requirement #349

jan-grimo · 2023-09-07T17:37:47Z

Completes isomorphism invariant improvements outlined in #203

Invariant contiguous range requirement removal

Vertex invariants for use in isomorphism algorithm must no longer have low upper bounds due to a hidden allocation linear in the maximum encountered vertex invariant.
Vertex invariants must no longer be convertible to size_t, but can be any comparable and hashable types
Build unordered_map-backed invariant multiplicity map efficiently from sorted vertex invariants

jeremy-murphy · 2023-09-08T01:16:02Z

Thanks for finishing this off!

I might not be able to merge this until I fix the modernization issues that are causing our CI builds to fail.

jan-grimo · 2023-09-08T07:28:41Z

Oh, I thought that was me! No worries, I'm not in a hurry.

include/boost/graph/isomorphism.hpp

jeremy-murphy · 2023-09-11T06:54:02Z

Btw, we generally put the { on a new line after if, etc, so please keep to that format.

jeremy-murphy · 2023-09-19T04:39:04Z

Can you merge develop into your branch to get the updated CI configuration please?

include/boost/graph/isomorphism.hpp

jeremy-murphy · 2024-04-03T00:17:05Z

I've forgotten where I was up to on this. :\

include/boost/graph/isomorphism.hpp

jeremy-murphy · 2024-04-05T05:36:38Z

include/boost/graph/isomorphism.hpp

-        typedef typename Invariant1::result_type invar1_value;
-        typedef typename Invariant2::result_type invar2_value;
+        typedef typename Invariant1::result_type invariant_t;
+        typedef unordered_map< invariant_t, size_type > multiplicity_map;


It's important to let the user choose this type, so it needs to become a template parameter with a default value, like:

template < typename Graph1, typename Graph2, typename IsoMapping, typename Invariant1, typename Invariant2, typename IndexMap1, typename IndexMap2, typename InvariantCountMap = boost::unordered_flat_map< typename Invariant1::result_type, typename graph_traits< Graph1 >::vertices_size_type > >

Note that I've used boost::unordered_flat_map, which generally blows boost::unordered_map out of the water. (See here.)

This is the only change left, then it's good to merge!

include/boost/graph/isomorphism.hpp

jeremy-murphy · 2024-04-08T05:16:06Z

@jan-grimo , do you still have time to finish this off? I'd like to get it done before I merge develop into master. I think there's just a bit of minor editing to be done.

jan-grimo · 2024-04-12T18:53:13Z

I don't understand what's wrong 🙈

2024-04-12T16:13:34.8397092Z D:\a\graph\boost-root\boost/unordered/detail/foa/core.hpp(1284): error C2146: syntax 
error: missing '>' before identifier 'key_type'
2024-04-12T16:13:34.8410724Z D:\a\graph\boost-root\boost/unordered/detail/foa/core.hpp(1289): note: see reference to class template instantiation 'is_map<Container>' being compiled
2024-04-12T16:13:34.8528000Z

jeremy-murphy · 2024-04-12T21:19:54Z

I don't understand what's wrong 🙈

2024-04-12T16:13:34.8397092Z D:\a\graph\boost-root\boost/unordered/detail/foa/core.hpp(1284): error C2146: syntax 
error: missing '>' before identifier 'key_type'
2024-04-12T16:13:34.8410724Z D:\a\graph\boost-root\boost/unordered/detail/foa/core.hpp(1289): note: see reference to class template instantiation 'is_map<Container>' being compiled
2024-04-12T16:13:34.8528000Z

Strange, I don't know either.

It may simply be that MSVC 14.0 is broken and I should take it out of the list.

jeremy-murphy · 2024-04-13T21:25:18Z

I removed it, so please merge develop into your branch and try again.

Invariant contiguous range requirement removal - Vertex invariants for use in isomorphism algorithm must no longer have low upper bounds due to a hidden allocation linear in the maximum encountered vertex invariant. - Vertex invariants must no longer be convertible to `size_t`, but can be any comparable and hashable types - Build `unordered_map`-backed invariant multiplicity map efficiently from sorted vertex invariants

Avoid requiring invariant default-constructibility - Refactor concept checking with boost type_traits

jeremy-murphy · 2024-04-15T01:19:35Z

Thanks for persisting with it, @jan-grimo !

jan-grimo · 2024-04-15T05:44:57Z

You're welcome! Thank you for helping me get it over the line @jeremy-murphy

jeremy-murphy · 2024-05-29T00:47:31Z

Are you able to quantify how the space efficiency changed? I mean, previously it was allocating a sequence container (vector) of size max_invariant, and now it allocates an associative container (hash map) of the same size as G1 -- did that change the overall space efficiency? Can you describe under what conditions the improved space efficiency occurs? I mean, what would previously cause max_invariant to be much greater than the size of G1 and thus incur a large allocation?

I just want to accurately document the change in the release notes. Thanks!

jan-grimo · 2024-05-29T05:53:41Z

Space efficiency hasn't changed for the general use case without custom invariants or the already feasible approaches with custom invariants that output a contiguous or small output domain on 0..N. But now arbitrary vertex invariants over vertex properties are feasible without incurring enormous memory allocations.

I had a use case with many vertex properties where I had set up a perfect hash function as an invariant, but then ran into the hidden memory allocation, which in the case of a 64-bit hash caused an OOM abort. I circumvented it by mapping the hashes for both graphs from their distribution over 0..2^64-1 down onto 0..N (where N is the number of unique hashes for the two graphs being compared). But I was irked by the hidden allocation and the odd invariant functor interface and thought it could be easier, hence the MRs that came from #203.

jeremy-murphy · 2024-05-30T23:45:20Z

Cool, thanks.

jan-grimo changed the title ~~Invariant contiguous range requirement removal~~ Isomorphism: Remove invariant contiguous range requirement Sep 7, 2023

jeremy-murphy self-assigned this Sep 10, 2023

jeremy-murphy reviewed Sep 11, 2023

View reviewed changes