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

Case Sensitivity - EDIF composer #182

Open
emonlux opened this issue Jul 1, 2022 · 10 comments
Open

Case Sensitivity - EDIF composer #182

emonlux opened this issue Jul 1, 2022 · 10 comments

Comments

@emonlux
Copy link
Collaborator

emonlux commented Jul 1, 2022

Unable to generate a TMR netlist
ValueError: Applying name would result in a naming conflict

namespace_manager -> init.py
Line 107 - if parent and parent in self.namespaces:

possible fix - if parent and element in self.namespaces:

@jacobdbrown4
Copy link
Collaborator

jacobdbrown4 commented Jul 7, 2022

I'm not super familiar with the namespace manager. However, line 107 seems like it's checking to make sure that parent is not None and that it's in self.namespaces.

@emonlux
Copy link
Collaborator Author

emonlux commented Jul 7, 2022

Yeah I eventually figured that out. Parent and Parent seems redundant though

@jacobdbrown4
Copy link
Collaborator

jacobdbrown4 commented Jul 7, 2022

So it looks like it's an issue with upper-case and lower-case. The namespace manager does not take case into account when checking for duplicate names. So F1_reg_TMR_0_Q_VOTER and f1_reg_TMR_0_Q_VOTER are considered the same. It's interesting that this wasn't caught when the instance was created but rather when composing the netlist.

This pull request #176 will attempt to fix the issue and give an option to support case sensitivity.

@jacobdbrown4
Copy link
Collaborator

It's also interesting that your generated netlist had the same names just different case. Where did the netlist come from?

@emonlux
Copy link
Collaborator Author

emonlux commented Jul 7, 2022

The netlist came from Vivado

@jacobdbrown4
Copy link
Collaborator

jacobdbrown4 commented Jul 7, 2022

Yeah I eventually figured that out. Parent and Parent seems redundant though

As for this, it may seem redundant but I don't think it is. It's checking to make sure the element has a parent, and then it's seeing if it's in self.namespaces. The writer may have done this because 'None' (which the parent would be if it didn't exist) may be in self.namespaces and may cause problems if we try to access it. So if the parent is 'None' it is passed by.

@jacobdbrown4
Copy link
Collaborator

That pull request looks like it gives the case sensitive option to the parser but not the composer. We probably need to add to it.

@emonlux emonlux changed the title Namespace manager bug Case Sensitivity - EDIF composer Jul 8, 2022
@jacobdbrown4
Copy link
Collaborator

See the case_insensitive_naming branch for some work on this.

@jacobdbrown4
Copy link
Collaborator

Note that this is related to issue #116

@wirthlin wirthlin added this to Summer Work in Fall 2023 Release May 15, 2023
@jacobdbrown4 jacobdbrown4 moved this from Summer Work to In Progress in Fall 2023 Release Aug 30, 2023
@jacobdbrown4 jacobdbrown4 moved this from In Progress to Summer Work in Fall 2023 Release Sep 8, 2023
@jacobdbrown4 jacobdbrown4 moved this from Summer Work to For a Later Release in Fall 2023 Release Sep 14, 2023
@jacobdbrown4
Copy link
Collaborator

Note: the case_insensitive_naming branch contains code to tell the EDIF parser to be case sensitive for naming. I merged this code into next release but then removed it, so github thinks this branch has nothing new but it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Fall 2023 Release
For a Later Release
Development

No branches or pull requests

2 participants