_assimilate_histogram and _regenerate_histogram refactor into standalone functions

Is your feature request related to a problem? Please describe.
_assimilate_histogram  and  _regenerate_histogram functions are not using self for anything of substance and as a result can be separated into their own standalone static functions.
Describe the outcome you'd like:
Move these two functions outside of the class to histogram_utils.py
Additional context:

I presume this would also involve writing unit tests for _assimilate_histogram and _regenerate_histogram in test_histogram_utils.py? Neither have any direct unit tests at the moment, though they might be called indirectly from other tests

Yes, definitely would want to ensure coverage with testing for sure @junholee6a

Working on this. It'll take a bit to understand the functions and write test cases

In _assimilate_histogram(), it seems like each bin in the from_hist maps to at most two bins in the dest_bin. This means that when from_hist has much larger bins than dest_bin, its values aren't distributed evenly:
Test code:
def test_assimilate_histogram(self):
    ret_value = histogram_utils._assimilate_histogram(
        from_hist_entity_count_per_bin=np.array([50, 0]),
        from_hist_bin_edges=np.array([0, 5, 10]),
        dest_hist_entity_count_per_bin=np.zeros(10),
        dest_hist_bin_edges=np.array((0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10)),
        dest_hist_num_bin=10,
    )
    print('=============== _assimilate_histogram result ===============')
    print(ret_value)

Output (reformatted by hand):
=============== _assimilate_histogram result ===============
(
    {
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 40.,  0.,  0.,  0.,  0.,  0.,  0.,  0.,  0.])
    },
    80.0
)

Expected output: bin_counts should be distributed more evenly
=============== _assimilate_histogram result ===============
(
    {
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 10.,  10.,  10.,  10.,  0.,  0.,  0.,  0.,  0.])
    },
    some_loss_value
)

Is this a bug?

Potentially related issue: https://github.com/capitalone/DataProfiler/issues/838

#838 is related. If we merge the capabilities and then make that merged function a standalone that would essentially solve both issues, but we separated these out as they accomplish different goals are are hefty tasks on their own for one developer.

In _assimilate_histogram(), it seems like each bin in the from_hist maps to at most two bins in the dest_bin. This means that when from_hist has much larger bins than dest_bin, its values aren't distributed evenly:
Test code:
def test_assimilate_histogram(self):
    ret_value = histogram_utils._assimilate_histogram(
        from_hist_entity_count_per_bin=np.array([50, 0]),
        from_hist_bin_edges=np.array([0, 5, 10]),
        dest_hist_entity_count_per_bin=np.zeros(10),
        dest_hist_bin_edges=np.array((0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10)),
        dest_hist_num_bin=10,
    )
    print('=============== _assimilate_histogram result ===============')
    print(ret_value)

Output (reformatted by hand):
=============== _assimilate_histogram result ===============
(
    {
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 40.,  0.,  0.,  0.,  0.,  0.,  0.,  0.,  0.])
    },
    80.0
)

Expected output: bin_counts should be distributed more evenly?
=============== _assimilate_histogram result ===============
(
    {
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 10.,  10.,  10.,  10.,  0.,  0.,  0.,  0.,  0.])
    },
    some_loss_value
)

Is this a bug?

Looking into this now

So there is a couple of things with this scenario that we cant necessarily make assumptions about.
While I agree that the actual result:
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 40.,  0.,  0.,  0.,  0.,  0.,  0.,  0.,  0.])
    },

Is not super intuitive, I also do not like the evenly distributed assumption:
{
        'bin_edges': array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10]),
        'bin_counts': array([10., 10.,  10.,  10.,  10.,  0.,  0.,  0.,  0.,  0.])
    },

because of the fact that there is a pretty big assumption there as well.
We are in both scenarios technically misrepresenting the data as its making the bins smaller and as a result forcing us to make those assumptions.
This is an open question within the team and we are definitely open to suggestions.
My personal thought is whatever we choose there needs to be a warning about losing accuracy if the bin sizes go from larger to smaller (less bins -> more bins).
@taylorfturner @micdavis @JGSweets @junholee6a thoughts?

because of the fact that there is a pretty big assumption there as well.

Definitely, this assumes that the values in from_hist are evenly distributed within each bin, but I don't think there's a more intuitive alternative without more information about the data represented by the histograms.

My personal thought is whatever we choose there needs to be a warning about losing accuracy if the bin sizes go from larger to smaller (less bins -> more bins).

Maybe this is covered by returning a higher hist_loss value? Accuracy can be lost when using _assimilate_histogram even if the bin sizes don't change, i.e. when the bins are only shifted sideways from from_hist to dest_hist.

After thinking about it, I may be leaning more towards your suggested expected outcome, I think your right in that there is not a more intuitive approach without some other significant details being shared.
If @taylorfturner and @micdavis are in agreement I can throw a bug report up to implement that fix instead of its current state, which IMO is worse than assuming the even distribution.


Using Realm in Cocoapods above iOS 7

I need to use Realm in my Pods but it only allow iOS above 8. I changed  to use static framework but ended up I got a lot of errors even if I followed the steps I use in my app
Error: embedded dylibs/frameworks only run on iOS 8 or later
"std::runtime_error::what() const"
"std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator >::compare(char const*) const", referenced from:

@amycheong19 can you please try removing the Realm framework from your project, and re-integrating by following the instructions at http://realm.io/docs/objc/latest/#installation? Thanks!

@amycheong19 I'm going to close this issue for now, but feel free to re-open if you're still running into trouble!


identify_windows script for image processing

this script takes in an image file path as an argument and asks gpt to identify the windows present in the image. from here we can prompt the user to select the window to crop, and then proceed another script of cropping.

merging windows script


Cue points for track 1 index 02+ are based on index 01

Version
20221217
Describe the bug
On Namco Video Game Graffiti Vol. 2, the split track cuesheet has track 1 indexes 02 and above at different time than in the whole disc image. They're off by 37 frames, which is index 01, so seems that the indexes are relative to index 01 instead of the beginning of the track. This seems to only be an issue on track 1 when index 01 is past 00:00:00.
This seems to be rare, I've not found another example of a disc like this.
Screenshots
tracks cue:
FILE "VDR-5222 (Track 1).bin" BINARY
  TRACK 01 AUDIO
    INDEX 00 00:00:00
    INDEX 01 00:00:37
    INDEX 02 00:11:40
    INDEX 03 00:37:33
    INDEX 04 03:11:33
    INDEX 05 03:40:68
    INDEX 06 06:07:28
    INDEX 07 06:35:50
    INDEX 08 07:52:45
    INDEX 09 08:33:33
    INDEX 10 10:11:38
    INDEX 11 10:54:55
    INDEX 12 12:55:43
    INDEX 13 13:49:40
    INDEX 14 15:17:53
    INDEX 15 19:33:48
    INDEX 16 24:40:38
    INDEX 17 25:34:50

img.cue:
FILE "VDR-5222.img" BINARY
  TRACK 01 AUDIO
    INDEX 00 00:00:00
    INDEX 01 00:00:37
    INDEX 02 00:12:02
    INDEX 03 00:37:70
    INDEX 04 03:11:70
    INDEX 05 03:41:30
    INDEX 06 06:07:65
    INDEX 07 06:36:12
    INDEX 08 07:53:07
    INDEX 09 08:33:70
    INDEX 10 10:12:00
    INDEX 11 10:55:17
    INDEX 12 12:56:05
    INDEX 13 13:50:02
    INDEX 14 15:18:15
    INDEX 15 19:34:10
    INDEX 16 24:41:00
    INDEX 17 25:35:12

Disc title
Namco Video Game Graffiti Vol. 2
Cat. no. VDR-5222
Disc ringcode
VDR-5222-1-A1E
URL
Amazon: https://a.co/d/beEJ4qY
Log file
VDR-5222_dic_test_20221217.zip

It's correct. MSF of tracks cue uses relative time, while MSF of img.cue uses absolute time.

sarami, you're correct with this statement.
But the problem here is that this disc TOC track 1 starts from lba 37 (index 1), but when you do a track split, you start with lba 0.
Due to 37 index 0 sectors prepended to Track 1, your index timepoints are all shifted 37 sectors in relation to track 1.
In the past, I had a discussion with other mods, where should track 1 split start if it's not LBA 0 in toc, and Fireball responded that we should always start from lba 0, which DIC does correctly, just the index entries have to be corrected.

Thanks sarami, let me try to explain better. I think the img .cue is correct to say index 1 goes from 00:00:37 to 00:12:02. But this is inconsistent with the track .cue, where index 1 goes from 00:00:37 to 00:11:40, unless some frames are missing from index 1 in the track .bin (I don't think that is the case).
For track 1, nLBAofFirstIdx is set to the start of index 1, but there's some index 0 sectors in track 1 on this disc. You have a special case to output indexes 0 and 1, but later indexes are relative to nLBAofFirstIdx, which is the start of index 1, past the start of the track.
So this only occurs when track 1 contains both index 0 and 2.

Thanks sarami, let me try to explain better. I think the img .cue is correct to say index 1 goes from 00:00:37 to 00:12:02. But this is inconsistent with the track .cue, where index 1 goes from 00:00:37 to 00:11:40, unless some frames are missing from index 1 in the track .bin (I don't think that is the case).
For track 1, nLBAofFirstIdx is set to the start of index 1, but there's some index 0 sectors in track 1 on this disc. You have a special case to output indexes 0 and 1, but later indexes are relative to nLBAofFirstIdx, which is the start of index 1, past the start of the track.
So this only occurs when track 1 contains both index 0 and 2.

MSF of tracks cue uses relative time

Ok, correctly, MSF of tracks cue uses absolute time from the each track, not relative time.

MSF of tracks cue uses relative time

Ok, correctly, MSF of tracks cue uses absolute time from the each track, not relative time.

test version
DiscImageCreator_test.zip

test version
DiscImageCreator_test.zip

That looks good for track 1!
But, now track 3 looks like this in the tracks .cue:
FILE "VDR-5222 (Track 3).bin" BINARY
  TRACK 03 AUDIO
    INDEX 01 00:00:00
    INDEX 02 35:11:47
    INDEX 03 37:21:47
    INDEX 04 40:37:65
    INDEX 05 41:14:55
    INDEX 06 41:47:50

It looks like index 2+ is absolute. I think these were correct before in 20221217:
FILE "VDR-5222 (Track 3).bin" BINARY
  TRACK 03 AUDIO
    INDEX 01 00:00:00
    INDEX 02 04:44:27
    INDEX 03 06:54:27
    INDEX 04 10:10:45
    INDEX 05 10:47:35
    INDEX 06 11:20:30

Thanks again, happy new year!

That looks good for track 1!
But, now track 3 looks like this in the tracks .cue:
FILE "VDR-5222 (Track 3).bin" BINARY
  TRACK 03 AUDIO
    INDEX 01 00:00:00
    INDEX 02 35:11:47
    INDEX 03 37:21:47
    INDEX 04 40:37:65
    INDEX 05 41:14:55
    INDEX 06 41:47:50

It looks like index 2+ is absolute. I think these were correct before in 20221217:
FILE "VDR-5222 (Track 3).bin" BINARY
  TRACK 03 AUDIO
    INDEX 01 00:00:00
    INDEX 02 04:44:27
    INDEX 03 06:54:27
    INDEX 04 10:10:45
    INDEX 05 10:47:35
    INDEX 06 11:20:30

Thanks again, happy new year!

fixed
DiscImageCreator_test.zip

fixed
DiscImageCreator_test.zip

This looks good for VDR-5222, thanks!
But the problem has moved to other tracks with index 0 and 2:
Battle Gear: The Discovery of Roads (ZTTL-0054, redump, Amazon) has index 0-3 on track 10. With the test above (20230101T151119, logs) this gives:
FILE "ZTTL-0054 (Track 10).bin" BINARY
  TRACK 10 AUDIO
    INDEX 00 00:00:00
    INDEX 01 00:03:22
    INDEX 02 00:12:39
    INDEX 03 02:43:12

I think it was correct before as:
FILE "ZTTL-0054 (Track 10).bin" BINARY
  TRACK 10 AUDIO
    INDEX 00 00:00:00
    INDEX 01 00:03:22
    INDEX 02 00:09:17
    INDEX 03 02:39:65

That was consistent with the img.cue:
  TRACK 10 AUDIO
    INDEX 00 17:49:05
    INDEX 01 17:52:27
    INDEX 02 17:58:22
    INDEX 03 20:28:70

Index 2 is 00:09:17 (692 frames) past index 0.

fixed
DiscImageCreator_test.zip

Looks good for everything I tested! VDR-5222 now seems correct, and everything else correctly matches the older versions.
Beyond VDR-5222 and ZTTL-0054 mentioned above, I tested INTIR-013 (Rockman 9 OST, many indexes including 0 on tracks 34 and 35) and PSCX-1005 (Final Fantasy III Legend of the Eternal Wind, index 1-2 on track 1). If you want the subchannel logs for any of these let me know.

If you want the subchannel logs for any of these let me know.

Yes, please.

https://shivermetimbe.rs/files/dumplogs/issue_172_logs.7z
These are all using the last DiscImageCreator_test.zip you posted here.