idofr/pymutohedral_lattice

meanings of `new_vals_idx`, `old_vals_idx`, `hash_table_base_idx`?

Closed this issue · 8 comments

Can you please comment on the meanings of new_vals_idx, old_vals_idx, hash_table_base_idx?

L422-429 doesn't seem to do anything.

Thanks!

idofr commented

Hey, I'll get back to you in the next few days.

idofr commented

Alright, so basically what I did was to translate the C++ code into Python in order for me to understand the algorithm better.
The original algorithm uses a lot of pointers to specific places in the arrays. In order to keep to the original structure of the code, I've replaced the pointers with matrix indices. The old and new indices directly affect the blurring. As for the base_idx I must admit I'm not sure anymore.

To do convolution, usually a new data structure is allocated to store the filtered values. I guess here the goal is use the same storage (the hashtable) for both old and new values, and relying on these index offsets to separate the old and new ones.

At its current form, I think your code simply overwrite old value with filtered new ones. This is done in a sequential manner, and thus doesn't exactly do the correct convolution.

idofr commented

The hashtable simply stores the structure of the lattice and is not changed after initialisation. The var for the filtered data is "new_vals" and the unfiltered data is "old_vals". These are then filtered based on the structure of the hash table.

Furthermore the implementation was validated against the original implementation. So I might have a bug or two there, but it's nothing major

Yes using 'new_vals' and 'old_vals' for filtered and unfiltered data should work. However these are not currently in the code, and the only array for storing values is hash_table.values in a single hash table.

idofr commented

Hm.. very odd indeed. I'll take a deeper look tomorrow

idofr commented

Hey, sorry for the exteremly late response. I did a quite fix on that one and it seems to be more sound that way, although almost no affect on 'Lenna'.

Thanks! I will close the issue for now and report later if I see any problems.