fix: fixing full auto flow#646
Conversation
There was a problem hiding this comment.
Pull request overview
Reworks the “full auto” FABulous fabric macro generation flow by improving tile-dimension exploration/optimization (including pin-based minimums), enhancing robustness of worker execution, and adding CLI/API knobs to run NLP-only or reuse prior exploration summaries.
Changes:
- Add pin-track-based minimum tile dimension tracking and propagate it into exploration summaries for NLP bounds.
- Refactor the NLP tile sizing formulation to use row-height/column-width variables with area-based feasibility constraints and configurable margin.
- Add CLI/API options to skip Step 1 via
--tile-opt-info, run--nlp-only, and configure--nlp-area-margin; update flow plumbing and final-views handling.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/gds_flow_test/step_test/conftest.py | Fix fixture docstring typo; extend mock config with pin-min fields. |
| tests/gds_flow_test/flow_test/test_full_fabric_flow.py | Update worker result typing/expectations to include pin-min payload. |
| fabulous/fabulous_cli/fabulous_cli.py | Add CLI args for cached tile-opt info and NLP-only/margin; pass through to API. |
| fabulous/fabulous_api.py | Extend full_fabric_automation() to accept NLP-only and margin, and pass into flow config. |
| fabulous/fabric_generator/gds_generator/steps/while_step.py | Persist final step config after loop completes. |
| fabulous/fabric_generator/gds_generator/steps/tile_optimisation.py | Rework iteration sizing logic; add pin-min config vars and routing-obstruction refresh helper. |
| fabulous/fabric_generator/gds_generator/steps/global_tile_opitmisation.py | Major NLP refactor: row/col variables, new JSON parsing, new constraints/metrics, updated termination. |
| fabulous/fabric_generator/gds_generator/flows/tile_macro_flow.py | Compute/store pin-min die constraints from IO track requirements; tighten DIE_AREA validation. |
| fabulous/fabric_generator/gds_generator/flows/full_fabric_flow.py | Worker returns pin-mins + state recovery on exceptions; add NLP-only mode; symlink-based final_views setup. |
| fabulous/fabric_generator/gds_generator/flows/fabric_macro_flow.py | Change final views snapshot destination path to project Fabric macro final_views. |
| fabulous/fabric_definition/tile.py | Simplify port counting; revise get_min_die_area() to track-based minimums with defaults. |
| fabulous/fabric_definition/supertile.py | Update supertile min-die sizing to track-based formulation consistent with Tile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi, I can run the flow, but it would need a merge with #670, and possibly other recent PR. |
|
Thanks for testing this. Is everything looking optimised well enough? |
f222891 to
dff21e1
Compare
|
Sorry, I meant I want to run the flow and see if it works here. It is not running yet; I'm hitting the PDK path error: ERROR | InvalidConfig | The following errors were encountered: |
|
I have done the rebase, and this should work now |
|
I've launched the command with the following methodolgy: git clone of repo Individual tile optimization seems to complete successfully; from the Fabric/macro/RUN-XXX folder, the flow.log file contains: === Step 3: Recompiling tiles with optimal dimensions === However, the flows interrupts with Can I provide more intermediate log files to help debug the flow? |
|
If you do Is the tile actually being compiled, and does it have the GDS? Because the log looks contradictory, unless those are deferred. But this |
|
You were right; the error comes from a resolve on a pdk_root parameter. See the complete trace |
|
I just also ran the flow and steps 2 and three of the eFPGA macro flow look like this: I did not clone a fresh repo this time. but just pulled the branch into an existing version of your fork. Not sure if this causes some issues. @mtetrault does it report successful runs in step 2 for you? Or do you get similar results as I do? |
|
Something is also looking wrong, the LUT4AB optimal dimensions at 429x379 are too large to be true... Need to check where this regression is from |
|
@IAmMarcelJung No, step 2 also shows the same message as you. In step 1, some iterations fail due to XOR difference, while others indicate "no working state found after tile optimisation" @KelvinChung2000 I'm also still hitting the |
|
The way the tile compilation works is by launching a new process, and it seems that leads to an issue with how the path is resolved when it is in a separate process. I am still working on this. I am getting errors in step 1 about not being able to find a specific file in the PDK, not an XOR error. It seems like on 3 machines with little different setup is leading to 3 kinds of errors... |
|
Actually I also got the XOR differences in Step 1. I will now run the flow again with your latest changes. |
|
I've run the flow once more with the recent changes. Here are the main outcomes I see:
|
I also get these results and the gds is also generated. |
|
295x211, this is more like what I am expecting. Which tile is getting XOR error? |
Seems like all tiles are getting the XOR error? But for the size, are we not expecting something optimized? We manually achieved 216.48x215.46 before for the LUT4AB tile. |
|
I reran the flow with the recent changes; similar results for now, step 2 reports that tiles "never compiled successfully" in "find_min_height" mode. It completes with the following dimensions:\
However, the W_IO fails to recompile. The message says "errors but state was recovered, however it gives an error on missing gds/lef for the W_IO: Tile W_IO had errors but state was recovered: Tile W_IO missing required outputs (GDS or LEF) About the XOR message, I'm wondering if the message is not from failed attempts, perhaps masking attemps that did work out? |
|
Some of the min_width and min_height modes might never pass due to the extram dimension ratio. The XOR check is not a problem; we only exclude the result if no working state is found. https://github.com/FPGA-Research/FABulous/pull/646/changes#diff-53d162e590b1758591554897d9ea5e25fe687ee58b273f5c7495c78a889a033bR632-R638 But still, having the XOR error is not great, since this should not have happened in the first place. |
|
New run, gives a rectangular shape: Optimal tile dimensions: { |
3027675 to
cda07a9
Compare
|
With recent changes: Optimal tile dimensions: |
|
Nice, this matches what I am seeing. There will be some run-to-run variation, but it is within a reasonable range. |
Co-authored-by: Marcel Jung <132259776+IAmMarcelJung@users.noreply.github.com>
9c52b9d to
4766a5c
Compare
EverythingElseWasAlreadyTaken
left a comment
There was a problem hiding this comment.
LGTM, I'm currently running a test run, once this ran through we can merge.
I have fixed/reworked the optimisation flow. It will be nice if someone can run it as well to verify everything is working.