Update save_images.tcl

[chetanyagoyal]

Fixes #254. Updates the save_images.tcl file to accommodate for the changes made in Openroad-flow-scripts

[saicharan0112]

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

[saicharan0112]

@msaligane please merge this so that daily CI will take latest save_image.tcl file and run it with the latest openroad version.

[harshkhandeparkar]

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

[saicharan0112]

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

Why do you think this should not be run on the CI?

[harshkhandeparkar]

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

Why do you think this should not be run on the CI?

gui::show probably means that it opens a GUI window. Also the comment above says that it should only run if OpenROAD is compiled with GUI.

[chetanyagoyal]

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

Why do you think this should not be run on the CI?

gui::show probably means that it opens a GUI window. Also the comment above says that it should only run if OpenROAD is compiled with GUI.

@chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

I agree that this shouldn't be run on the CI. Don't think there's anything wrong with the commands per se. Maybe we could add a condition in the flows that asks it to bypass this statement?

[saicharan0112]

From the idea of the saving images and the GUI, it makes sense. But since openroad is not throwing any error even though the command is executed (remember that you cannot launch gui using this conda package) and the check is able to catch the command changes, I think we should keep it as it is.

On Mon, 30 Oct, 2023, 9:36 pm Chetanya, @.***> wrote:

@chetanyagoyal https://github.com/chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

Why do you think this should not be run on the CI?

gui::show probably means that it opens a GUI window. Also the comment above says that it should only run if OpenROAD is compiled with GUI.

@chetanyagoyal https://github.com/chetanyagoyal this does fix the issue. Its like chicken and egg problem.. Let this update get merged so that the docker image (stable tag) will get updated with the latest openroad.

https://github.com/idea-fasoc/OpenFASOC/blob/25a97ea8eb57669582ddf5ab944edf067a713805/openfasoc/generators/temp-sense-gen/flow/scripts/final_report.tcl#L60-L63

These lines (which call the script) suggest that this script should not be run on the CI. Is it configured improperly perhaps?

I agree that this shouldn't be run on the CI. Don't think there's anything wrong with the commands per se. Maybe we could add a condition in the flows that asks it to bypass this statement?

— Reply to this email directly, view it on GitHub https://github.com/idea-fasoc/OpenFASOC/pull/256#issuecomment-1785544253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYCOBKIPIFNQPZLYA3RZCCLYB7GAPAVCNFSM6AAAAAA6RP3D5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBVGU2DIMRVGM . You are receiving this because you modified the open/close state.Message ID: @.***>

[saicharan0112]

Thanks @chetanyagoyal . Tomorrow's check from the bot will help us test the latest openroad version with this update