Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Conversation

@William-Baker
Copy link
Contributor

No need to copy annotations, this is expensive

Copy link
Contributor

@david-lindner david-lindner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this does not improve the compilation performance but your program generation code? How large is the speedup?

I'm slightly hesitant about this change because it looks like something that might introduce silent bugs. I have to think a bit more whether something can break here.

If we make this change: we can also remove .copy and the dependency on copy.

@William-Baker
Copy link
Contributor Author

I've modified the doc string and removed the copy methods and dependency, at the time of removing this, it took around 30% of compilation time just on copy operations, however some were large programs

Copy link
Contributor

@david-lindner david-lindner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rasp_test currently fails -- pls fix before merging

@William-Baker
Copy link
Contributor Author

I have removed the tests for copy() and copying when annotating, we could always keep the copy method and keep the copy tests?

@david-lindner david-lindner self-assigned this Feb 7, 2024
Copy link
Contributor

@david-lindner david-lindner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still makes rasp_to_craft_integration_test fail in a bunch of places. Can you look into this?

Also, can you please run all the tests in the future before making a PR? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants