Add svs flow (No extraction) to klayout LVS

[H-Ojiro]

This is an enhancement

• Tests pass
• Appropriate changes to README are included in PR (run_lvs.py usage updated)

[sergeiandreyev]

Hi @FaragElsayed2, could you please take a look at this PR?

[FaragElsayed2]

@sergeiandreyev Approved, but I believe it would be better to add this switch also for the GUI options.

[sergeiandreyev]

Hi @H-Ojiro, what do you think to provide this option in GUI as well, is it a reasonable enhancement?

[H-Ojiro]

Hi, @sergeiandreyev

Thank you for your comment.
I think adding svs option into GUI menu would be helpful.
The option might be installed in combo_box list rather than check_box item
as following image;

@d-m-bailey

[sergeiandreyev]

@atorkmabrains, @FaragElsayed2, your view on this? for me it's fine

[FaragElsayed2]

Hi @sergeiandreyev

I believe it's fine with keeping it in the same Run Mode menu, since the netlist will already be extracted with the desired mode.

[atorkmabrains]

@H-Ojiro What's the intent of this change? I see the switch but I'm not sure what is the target that you are trying to achieve.

[atorkmabrains]

@sergeiandreyev To me this change looks like it stops Klayout from generating a layout netlist only. I'm not sure of how it functions:

run_lvs.py (--layout=<layout_path>) (--netlist=<netlist_path>)
    [--run_dir=<run_dir_path>] [--topcell=<topcell_name>] [--run_mode=<run_mode>]
    [--no_net_names] [--spice_comments] [--net_only] [--no_simplify]
    [--no_series_res] [--no_parallel_res] [--combine_devices] [--top_lvl_pins]
    [--purge] [--purge_nets] [--verbose]

These are all the switches that the script takes:

• --netlist : provides the schematic netlist for comparison.

I don't see an addition for the layout netlist.

If I understand the word "SVS" ---> "Schematic Vs Schematic", I believe this code most likely will not do that.

You will need to add another switch with a path for the layout_netlist, may be call it "--layout_netlist" not use the same as the schematic netlist.

[atorkmabrains]

@H-Ojiro Could you clarify?

[atorkmabrains]

@H-Ojiro @sergeiandreyev This will break the original LVS as well.

[H-Ojiro]

Hi, @atorkmabrains,

I've modified sg13g2.lvs to prevent breaking the flow as follows;

if $target_netlist
  logger.info("LVS extracted netlist at: #{$target_netlist}")
  netlist_path = $target_netlist
  if ! SKIP_EXTRACT
    target_netlist($target_netlist, custom_spice_writer,
                   "Extracted by KLayout with SG13G2 LVS runset on : #{Time.now.strftime('%d/%m/%Y %H:%M')}")
  end
else
  layout_dir = Pathname.new(RBA::CellView.active.filename).parent.realpath
  netlist_path = layout_dir.join("#{source.cell_name}_extracted.cir")
  if ! SKIP_EXTRACT
    target_netlist(netlist_path.to_s, custom_spice_writer,
                   "Extracted by KLayout with SG13G2 LVS runset on : #{Time.now.strftime('%d/%m/%Y %H:%M')}")
  end
  logger.info("SG13G2 Klayout LVS extracted netlist file at: #{source.cell_name}_extracted.cir")
end

[atorkmabrains]

@H-Ojiro Could you share a working test case to validate that this is working?

[atorkmabrains]

@H-Ojiro The changes you have made don't have any test case added, could you add a small test case in the actions to validate it's working? You could add a new actions file separate from the original one and add your test case in the testing folder.

[atorkmabrains]

@H-Ojiro If your intent is to stop the extraction of the layout netlist, I would recommend that you don't add it to the drop down menu as @FaragElsayed2 has mentioned, and you add it as a switch. As I pointed out, this will most likely break the original flow.

[d-m-bailey]

@H-Ojiro What's the intent of this change? I see the switch but I'm not sure what is the target that you are trying to achieve.

@atorkmabrains
The title of the pull request "Add svs flow" is a misnomer and as you mentioned, would require a --layout_netlist to handle generic cases.

The purpose of this change is to add the option to run LVS on an already extracted layout. Layout extraction requires the most time and is not necessary if only the rule files or schematic netlist changes.

[atorkmabrains]

@H-Ojiro What's the intent of this change? I see the switch but I'm not sure what is the target that you are trying to achieve.

@atorkmabrains The title of the pull request "Add svs flow" is a misnomer and as you mentioned, would require a --layout_netlist to handle generic cases.

The purpose of this change is to add the option to run LVS on an already extracted layout. Layout extraction requires the most time and is not necessary if only the rule files or schematic netlist changes.

@d-m-bailey If that's the case, I believe the code won't do this. It's close, but is not exactly correct. Such change would require proper switch testing as well.

@H-Ojiro Please add a test case to validate your changes. The test case should have 2 steps verification at least:

1. You should make sure that normal operation is still functioning if the user didn't use that switch.
2. You should add a 2 or 3 test patterns:
   • 2 matching netlists. [You must verify it's matched in your testing]
   • 1 master netlist and one with connection failure. [You must verify it's not matching in your testing]
   • 1 master netlist and one with parameter failure. [You must verify it's not matching in your testing]

If you are willing to go the extra mile, I would recommend running the above 3 test cases with different netlist handling switches to understand the impact as well.

@H-Ojiro I would like to thank you for your contribution regardless. I understand I might sound harsh. But I truly appreciate your contribution here.

[H-Ojiro]

Hi, @atorkmabrains

Please find attached tar file: testing_svs.tar.gz which contains LVS/SVS testcases for running test on local Git repository
Refer to README file in the tar file

testing_svs.tar.gz

[H-Ojiro]

Following @atorkmabrains 's recommendation, changed SVS option to 'Compare Only' switch box on LVS Options for GUI

[atorkmabrains]

@H-Ojiro Appreciate your support and patience on this, Could you please add a new action flow that would run the tests that I have listed above?

[d-m-bailey]

@atorkmabrains Thanks for the feedback. Just to be clear, there are currently no actions that test for expected LVS failure. I believe they all test that LVS passes, right?

However, you want us to add an action just for SVS that fails? There is already an action for SVS passing.

[atorkmabrains]

@d-m-bailey @H-Ojiro
I could see the test now, but I believe there are multiple changes to make sure it's complete. You need to add 2 sample netlists in the testing folder one for schematic and one for layout at least and if you want to complete the testing you need to add the examples that I have mentioned above.

I believe you need to add another input switch called: layout_netlist that provides the netlist path for the layout and you could ignore the "skip_extraction" entirely. If this is defined, then we use the provided netlist rather than the layout.