Library Logic Format Simplification #1365
Conversation
b-shi
requested review from
jichangjichang,
KKyang,
aazz44ss,
vin-huang,
imcarsonliao,
hcman2,
Serge45,
Jinp800125,
TonyYHsieh and
solaslin
as code owners
|
why do we need to keep merge.py? Should we just update merge.py for new format? |
| @@ -320,11 +343,10 @@ def parseLibraryLogicList(data, srcFile="?"): | |||
| .format(srcFile, len(data))) | |||
|
|
|||
| rv = {} | |||
|
|
|||
| rv["LibraryLogicVersion"] = "0.0.0" | |||
There was a problem hiding this comment.
We hard code the library logic version to 0.0.0? Should this instead be extracted from the library logic directly?
edit: I see, the original list-style logic files won't have a version; disregard, or perhaps a comment could be useful.
There was a problem hiding this comment.
Yeah, parseLibraryLogicList can be removed once all the logic files are in the new format. Currently its only being used to help convert the legacy format to new format.
| if isinstance(data, List): | ||
| # TODO: this can be removed when all logic files have dict format | ||
| data = parseLibraryLogicList(data, srcFile) | ||
| else: | ||
| libraryType = data["LibraryType"] |
There was a problem hiding this comment.
I wonder if here on the else branch, we should convert it into an elif and check against the expected type (Dict) and the library logic version, then add an else block that raises if nothing matches?
There was a problem hiding this comment.
Thanks, good point, I will have it check the LibraryLogicVersion and type
|
I don't think we need a mergeV2.py, we should let merge.py to support the new format and make sure it also supports transferring legacy yamls to new format. |
@babakpst, can we replace the old |
Just to check, if we convert all the existing logic files to the newer format, would we still have a need to support the legacy format? One of the motivations for converting all the logic files to the new format in this PR is to avoid adding support for the legacy format (main issue is there a several places where the fields are hard coded). |
|
The reason why we still need a converter is that customers are using our tuning guides and having their own logic yamls kept in their hands. This change without a converter will break the compatibility. |
Ah.. thats a good point, thoughts on adding a convert option in |
This PR makes a few changes to the library logic format and related for simplification purposes:
ProblemTypefield in each solution - the global one is used insteadDefaultSolutionfield in each logic file.DefaultSolutionin each logic file to fill in the missing parameter valuesLibraryLogicVersionto track any logic file format changes in the future.mergeV2.pyfor merging logic files in new format .With the format changes the total disk usage of the logic files is reduced ~40%. Sample logic file using new format here.
Tensilite changes are in a separate commit for ease of review.