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

Remove dangling files, update parsing scripts across generators and workflows #251

Merged
merged 50 commits into from
Nov 12, 2023
Merged

Conversation

chetanyagoyal
Copy link
Collaborator

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

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
@chetanyagoyal chetanyagoyal changed the title Remove hanging files Remove dangling files Oct 11, 2023
@chetanyagoyal
Copy link
Collaborator Author

@saicharan0112 @msaligane

@chetanyagoyal
Copy link
Collaborator Author

@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?

@saicharan0112
Copy link
Collaborator

@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?

@chetanyagoyal
Copy link
Collaborator Author

@saicharan0112 are there any other changes required before this is merged?

Copy link
Collaborator

@saicharan0112 saicharan0112 left a 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.

openfasoc/generators/temp-sense-gen/tools/parse_rpt.py Outdated Show resolved Hide resolved
.github/scripts/parse_rpt.py Outdated Show resolved Hide resolved
.github/scripts/parse_rpt.py Outdated Show resolved Hide resolved
.github/scripts/parse_rpt.py Outdated Show resolved Hide resolved
.github/scripts/parse_rpt.py Outdated Show resolved Hide resolved
openfasoc/generators/PMU-gen/tools/parse_rpt.py Outdated Show resolved Hide resolved
openfasoc/generators/common/__init__.py Show resolved Hide resolved
openfasoc/generators/dcdc-gen/tools/parse_rpt.py Outdated Show resolved Hide resolved
openfasoc/generators/scpa-gen/tools/parse_rpt.py Outdated Show resolved Hide resolved
openfasoc/generators/temp-sense-gen/tools/parse_rpt.py Outdated Show resolved Hide resolved
.github/scripts/parse_rpt.py Show resolved Hide resolved
Copy link
Collaborator

@saicharan0112 saicharan0112 left a 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

.github/scripts/parse_rpt.py Outdated Show resolved Hide resolved
openfasoc/generators/common/__init__.py Show resolved Hide resolved
@chetanyagoyal
Copy link
Collaborator Author

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

Copy link
Collaborator

@saicharan0112 saicharan0112 left a 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.

@saicharan0112 saicharan0112 merged commit 28770f7 into idea-fasoc:main Nov 12, 2023
4 of 8 checks passed
@saicharan0112 saicharan0112 changed the title Remove dangling files Remove dangling files, update parsing scripts across generators and workflows Nov 12, 2023
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.

3 participants