Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PDL::IO::Dumper: better handling of multiply referenced ndarrays #509

Merged
merged 9 commits into from
Dec 15, 2024

Conversation

shawnlaffan
Copy link
Contributor

  • Keep a track of which ndarrays have already been dumped to avoid duplicating them in the dump string.
  • Handle multiple refs in the dumper structure.
  • Generalise dumper tests to handle future changes to dump method thresholds.

Includes some modernisation of code. More could be done but that's better as a separate PR.

Fixes #508

Currently a structure like this returns undef for duplicated
entries, as well as raising "my variable masking" warnings
when evaled back.

my $p = zeroes (5);
my $arr = [$p, $p];
This also avoids "my variable masking" warnings
when eval'ing smaller ndarrays back to a structure.

Warnings for larger uuencoded strings are not yet handled.
It interferes with uuencoded strings.

Temporary change.
* Use named lexical as the loop variable
* Unpack all args at start
* Some whitespace changes
Shift most of the work to an internal sub
which works with an array instead of strings.
This avoids problems with deduplication when
structures span more than one line.
This allows us to pass the seen hash as an arg
and dedup as we go.  Should save memory for large
structures.
Don't serialise an ndarray that has already been processed.

This avoids the need to deduplicate later,
although that process is retained just in
case it is useful.
The threshold for using uuencode may change in future
given 25x25 is not that large these days.

Do the same for the small threshold used to
determine if the struct should be inlined.
@coveralls
Copy link

Coverage Status

coverage: 66.844% (+0.05%) from 66.79%
when pulling 5d3e3fe on shawnlaffan:io_dumper_dups
into 5c7294d on PDLPorters:master.

@mohawk2
Copy link
Member

mohawk2 commented Dec 15, 2024

Superb, thanks! Seeing the test code tells me I need to adjust that to use Test::PDL as well.

@mohawk2 mohawk2 merged commit 77e1fd4 into PDLPorters:master Dec 15, 2024
78 of 80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PDL::IO::Dumper frestore doesn't resolve duplicated ndarrays
3 participants