Skip to content

IOfile refactor#17

Merged
vlahtin merged 38 commits intov1.14_RCfrom
iofile-refactor
Dec 1, 2025
Merged

IOfile refactor#17
vlahtin merged 38 commits intov1.14_RCfrom
iofile-refactor

Conversation

@mkosunen
Copy link
Copy Markdown
Member

@mkosunen mkosunen commented Apr 15, 2025

Should address issues:
#16
#18
Ping @sporrasm @vlahtin

mkosunen and others added 4 commits April 15, 2025 17:59
Ordering was incorrectly reversed
- Crashes were due to missing trigger signal that was not read to
  self.iofile_eventdict, since trigger signal was not read unless
  explicitly defined to be an event type output

- On the other hand, explicitly defining triggers as event outputs
 caueses columns to overflow in the print file (i.e. num_cols is
 calculated incorrectly) in some cases

- The most convienient solution is to just read the output file once
BEFORE reading the individual IOs
@sporrasm
Copy link
Copy Markdown
Contributor

sporrasm commented Aug 8, 2025

Fixed #22 by refactoring print-file reading away from the spice_iofile, since it is not related to any single IO, but all of them. This may break other things, I guess.

Also, fix #16

@vlahtin
Copy link
Copy Markdown
Contributor

vlahtin commented Aug 15, 2025

Tried AC; does not work yet. The reason is that on line 180, the io type is read and it uses tran_analysis_name always for event based results (even though AC can also use those).

Need to implement some method to extract whether or not the simulation was actually AC or Transient. This information is atleast given to spice_simcmd, so have to figure out some method to get it here as well. Did not have time today to debug this further, but leaving this as documentaiton if someone else has the time to figure this out before me.

@sporrasm
Copy link
Copy Markdown
Contributor

I see. The way I see it is that we three options

  1. Either use the same analysis name for all simulations, which is probably not a good idea
  2. Infer output file name from properties of spice_iofile. For example, I think AC simulations should have iotype='event' and datatype='complex'
  3. Detect which simulation type we are running and infer output file name based on that, which is the same what @vlahtin proposed above.

To me option 1 is the worst and option 2 is not very explicit and hence might be difficult to debug. It probably makes sense to detect the simulation type and infer analysis name from there directly. This will mean that running two different simulations on the same netlist will not work, but I don't think that has been supported for a long time anyway.

@vlahtin
Copy link
Copy Markdown
Contributor

vlahtin commented Aug 18, 2025

da34a25 should fix the aforementioned problem.

@sporrasm
Copy link
Copy Markdown
Contributor

sporrasm commented Aug 18, 2025

Using libpsf to read in transient and AC. Tested with transient. This now has the downside that sparam, etc, still use psf_utils, which we should probably get rid of, if libpsf is superior to psf_utils. See #25

@vlahtin can you test and see if it is faster now? Please remember to pip3 install --user libpsf

@sporrasm
Copy link
Copy Markdown
Contributor

sporrasm commented Aug 18, 2025

Tested also AC with a nonsense example. At least it doesn't crash.

We should probably aim to make release tests for all analysis types to aid with the testing.

Santeri Porrasmaa and others added 12 commits August 19, 2025 09:01
- This required to get rid of iotypes 'psfascii_pss' and 'psfascii_pac',
which are simulator specific. They are now handled as events. The
reason is that AC simulation output (freq, amplitude pairs) is also an event.
PSS and PAC output similar type of data. The frequency is fundemntal or
harmonic in PSS and in PAC it is fund +/- nharmonics * pss_fund.

- Seems to break noise simulations...
@sporrasm
Copy link
Copy Markdown
Contributor

sporrasm commented Aug 20, 2025

Okay, tested this together with @MiikkaMaki. transient, dc, ac, sp, noise, pss and pac simulations seem all to work. Of course, further testing would always be nice to catch possible uncaught bugs (mention @vlahtin if you have the time).

iofile reading operation should now be simulator agnostic once again, which is good news.

@vlahtin
Copy link
Copy Markdown
Contributor

vlahtin commented Sep 8, 2025

Added the option to select either binary parsing (using libpsf) or ascii parsing (using psf_utils) when use_psf=True. Also made it so, that psf_utils is used for all other analyses (when use_psf is True), as the output file size is not expected to grow as significantly as transient, thus yielding reliable results with reasonable speed.

This change came as libpsf has problems with memory leakage with large output file sizes, and unavailable in some linux distributions. The psf_utils is however quite slow, so if large file sizes in transient are expected, either libpsf or print file parsing may be the best option, which should now be available all according to user selection

@MiikkaMaki can you test that your SP, noise, PSS, PAC testbench still works?

@vlahtin
Copy link
Copy Markdown
Contributor

vlahtin commented Nov 28, 2025

Hi,
happened to do something that has not been previously done with Spice. Lets add the fix to that to this merge as well:
#29

Please test that as well before merge. It works in my special case, but if someone has another case with sample IOs, please try that.

@mkosunen
Copy link
Copy Markdown
Member Author

mkosunen commented Nov 29, 2025

Notes for the release:As the lint-based code reformat policy has been added to v1.14_RC already, I suggest that you do octopus merge (and some tests) by merging v1.14_RC to this branch and re-test before merging this to v1.14_RC, if not done already.

@mkosunen
Copy link
Copy Markdown
Member Author

mkosunen commented Nov 29, 2025

Note for the future developments: If there is a need for a iofiletype where the first column is frequency, (for example in AC-analysis) then the most consistent way to handle this is to add io type 'freq' with an assumption that the data in the other columns are complex. All the other formats address the problem how the time in the first columns is interpreted.

Opened #31 for this, does not need to be addressed now.

Also: AC response is not a 'signal' as in sense of conveying data from a block to another and thus not strictly an IO. To me it seems that it should be handled with some kind of 'extract' rather than IO's. Freq type signal would be for example an OFDM frame.

@mkosunen
Copy link
Copy Markdown
Member Author

mkosunen commented Dec 1, 2025

Further notes fro the release: To reduce the number of conflicts at merge, regrdless of which way it is done first, you may run
black -l 80 --check --diff --color <list of python files>
to check the syntax differences and then correct them with
black -l 80 <list of python files>

@vlahtin vlahtin merged commit 28e2a25 into v1.14_RC Dec 1, 2025
@mkosunen mkosunen deleted the iofile-refactor branch December 23, 2025 12:25
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.

4 participants