Skip to content

Conversation

@robin-tukl
Copy link

Hey,

I found an error if you try to read large PSF data. Tried to fix it the same way as explained in henjo/libpsf@61d8c0f . For me it worked.

Best regards
Yannick

@ma-laforge
Copy link
Owner

Hi,
I'm glad you are looking into Julia to do your data processing :).

I want to take a better look at what you are changing. I don't quite understand what the patch does - and I noticed you did not merge your patch into Henrik's stream (though he doesn't appear to have changed much since 2014). I also noticed your C++ solution appears much more elegant. I wonder if I can get the Julia changes to look as good.

It might be a bit easier for me to if I knew what I was fixing. For example:

  • at what file size does the current solution break?

I do appreciate the fix and will try to get it integrated when I get some time.

@ma-laforge
Copy link
Owner

I think I understood what you were trying to do, and added a similar solution to the master branch.

Notes:

  • The solution looks a bit more like your patch for Henrik's stream.
  • I did not change to UInt32 for tmp = read(r, Int32). I want to keep to Henrik's solution unless something is broken, and the following line (n = tmp & 0xFFFF) really ensures we get a positive integer anyways.

Please verify that this solution works for you. I will create a new release if it does.

@robin-tukl
Copy link
Author

Hey,
This does not do the trick for me. I still get the incorrect chunk error. I guess it is essential to treat the chunk with id 20 different from the chunk with id 16. I want to upload an example file which is crashing, but they are too big. Is there another way to send you this transient file?

@ma-laforge
Copy link
Owner

Sorry, I was unavailable for a few days.

Hmm.. True, but the new function does differentiate between the two chunks. It does so through definitions that result in the following:
chunkid(ZeroPad)==20, whereas chunkid(SweepValueWindowed)==16

It is just better form to not hard-code these values in.

-> But I might have made a mistake reverse-engineering your solution. There appeared to be a fair amount of unused code. The solution you proposed to henjo's stream was much more setreamlined.

Questions

  • Are you sure you tested with the master branch? I have not yet tagged this version. You could even add your own print(:AMIMASTER) line after deserialize_chunkpadded just to make sure you have the correct version.
  • Is it possible that even the first chunk (i==0) will have a chunk id of 20? If so, that is where my error would lie. I could not tell for sure without a sample dataset.
    • If that's the problem, you can test the solution by replacing: deserialize_chunkpadded(r, SweepValueWindowed, 0==i) with deserialize_chunkpadded(r, SweepValueWindowed, false)
  • You could also print out the value of i when you detect that the chunkid is 20 (in addition to totaln)! That might be useful.

Sample file

If I understand correctly, this issue is not really about total file size. I suspect that your file writer leaves zero padding for no good reason. My own file writer (PSFWrite.jl) never puts zero padding for SweepValueWindowed. It was written this way.

Maybe try to generate a dataset (run a simulation?) where the number of data points is no more than 2 or 3 of the window sizes (windowsize = Int(r.properties["PSF window size"])) used by your data writer? If you can send me a small test file, we might be able to generate a fix that I don't mind merging in the main stream.

@robin-tukl
Copy link
Author

I'm pretty sure I tested the correct master branch as the error is different than before, but here is a sample file to test for my case (https://drive.google.com/open?id=12MbQNTL5VRCsUhsmZm3I1nwhp99S91pE). The error occurs while reading out the timing (readsweep()) or one net (read('',"CK")). I think now something is wrong with the pointer.

About the situation, with the very first chunk, I'm not sure.

The data is directly generated from a SPICE simulation and I think that there is unnecessary zero patting.

@ma-laforge
Copy link
Owner

Ok. I can now read your file no problem. It seems like you must re-read the chunkid after skipping over the padded data.

Can you confirm everything is ok at your end?

I initiated the process of tagging a new version, but it might take a while to make it into Julia's Registry.

Sorry for the delay.

@ma-laforge
Copy link
Owner

Ok. Everything should be properly tagged now.

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.

2 participants