f4pga-sdf-timing
f4pga-sdf-timing copied to clipboard
chipsalliance
Updates and improvements to SDF parser.
This pull requests makes quite a few changes to the original code. The list of major updates:
- Removed parser internal helper collections. The updated parser now returns the parser structure as a return value of grammar rule expansions. This change simplifies the code and would enable future refactoring to the visitor pattern (for greater flexibility of the parser).
- Changed internal structure of parsed data to better represent input data 1:1. The new structure keeps any duplicities if they existed in the input SDF and it preservers order of SDF timing records. These changes are to better correlate emitted parsed SDF to its original input.
- Added support for more SDF constructs (e.g.
RECREM,PERIOD,RETAIN), incl. SDF comments. The comments are handled by the lexer as an ignored token and hence does not presently propagate to the parser. - Fixed semantics of
delval_listto correspond to SDF 3.0 Std. - Added unit tests of individual SDF syntax elements. The unit test is however far from complete.
- Added unit tests of complete SDF parsing and writing out the parsed data back to SDF. This is much like the original
parse_all_test.pyexcept thattest_parse_and_write.pyoperates on SDF strings and not files. - Added few simple utilities, e.g. to convert SDF to JSON or to reformat SDF.
This update breaks some compatibility with the original code. This makes the parse_all_test.py test suite fails as some
of the golden files no longer correspond to how the parsed data looks like now, when emitted into a file. To make the test
pass again, new golden file set is to be generated and reviewed for expected contents. This is something I ceased to
do as my expectations of the output data may be different than that of the original authors.
@brabect1 thanks for the PR!
I've just merged https://github.com/chipsalliance/f4pga-sdf-timing/pull/52, fixing the CI. Can you rebase on top of that so we can run the tests here?
Hi Karol, I do have little experience with collaboration on Github and will need a bit of guidance. I rebased master in my fork. Am I supposed to start a new PR now?
@brabect1 - You just push over the previous branch and it will update the pull request -- seems like you figured that out?
there seems to be a number of python errors like e.g.:
E ImportError: attempted relative import with no known parent package
Hi guys, I will need more guidance on how to fix these CI workflows. Say a make a change that is assumed to fix a problem. I guess I have to push into master branch. How do I run the CI tasks then to see if the fix worked?
Hi, can someone trigger CI to see if there are more things to fix? thanks
CI was triggered and finished, but it is still red - mostly in package tests:
E ImportError: attempted relative import with no known parent package
Hi Karol, I did one more fix. Could you please trigger CI again? I am not able to reproduce the issue locally, which is why this trial and error approach ...
Ok, looks good. Now the interesting part. parse_all_test.py is failing because it now produces a different output than in the reference data set. For example, the original parser code removed duplicate SDF records; the new code keeps the duplicities as it is not up to the parser to resolve it (it is rather up to the consumer of parser output).
So I guess someone from the team should review, if the new output is ok. You can either generate the new reference set yourself, or I can make it a part of the pull request (replacing the old reference set). Which way you prefer?
Hi guys, as no one responded to my last post I made updates to the golden set files to match what the parser now generates. Can you trigger CI so we see there are no other issues? You can then review the changes to the golden set. Thanks.