-
Notifications
You must be signed in to change notification settings - Fork 109
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
Remove dangling files, update parsing scripts across generators and workflows #251
Conversation
Recreated functionality in ./parse_rpt.py
remove verify_op.sh calls
Update to include checks for .csv files and ../work directory files
Add checks for .csv files and ../work directory files (.gds, .pex, .v, .spice, etc.)
Added print statements
Added print statements
Added necessary checks for generated .csv files and files in the ../work directory (.gds, .spice, .v, etc.)
Added ../work directory and .csv file checks, commented because only the verilog make is supported
the if condition checks "content1" against itself when it should check against "content2"
@saicharan0112 As the ldo-gen tools folder does not have a parse report file, would you suggest that I make one along the lines of what the .github/scripts parse_rpt.py file does? |
I think its a good idea to have all generators follow the same format to check for drc and lvs errors. tbh I would expect a function inside the main script itself but for now it is ok to have a different file. Here is a suggestion.. can you list all files that are generated by the three generators (cryo-gen, ldo-gen and temp-sense-gen) and see if the existence of these files are checked along with the drc and lvs errors please? |
@saicharan0112 are there any other changes required before this is merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of code redundancy. I think you can have one single function for these checks (probably you can use the common directory outside the generators?) and reuse with generator specific files to do checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope you are working on logic summary in the beginning of parse_rpt.py
under .github/scripts
and common/check_gen_files.py
Addressed all issues, please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chetanyagoyal . It can still be improved but it looks very good for now. Right now cryo-gen and ldo are not reaching till the stage where the reports are checked and the outputs are verified, so let's see how it goes once they are active.
This PR attempts to remove hanging files (like ./openfasoc/generators/temp-sense-gen/tools/verify_op.sh) and includes their functionalities in the corresponding report parse files
Also does the same for the parse_rpt.py file found in the .github/scripts section to account for this functionality in the CI