Skip to content

Conversation

@lowasser
Copy link
Collaborator

@lowasser lowasser commented Nov 8, 2025

I started out wanting to make some parameters implicit, now that with opaque definitions they can reliably be inferred from the context.

Then things snowballed from there to include a bunch of #1545, use of abstract opaque, and elimination of redundant imports.

@fredrik-bakke
Copy link
Collaborator

Have you made any measurements wrt. the performance impact of these changes? One thing that worries me is that, even though Agda may be able to infer certain arguments, doing so may take nontrivial amounts of typechecking time and memory. This for instance led to the recent memory issue #1577. Though, in that case nothing was marked abstract or opaque, so your changes might work well with Agda.

Copy link
Collaborator

@fredrik-bakke fredrik-bakke left a comment

Choose a reason for hiding this comment

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

Conditional approval, depending on that the performance is not drastically worse than before.

Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
@lowasser
Copy link
Collaborator Author

lowasser commented Nov 8, 2025

Have you made any measurements wrt. the performance impact of these changes? One thing that worries me is that, even though Agda may be able to infer certain arguments, doing so may take nontrivial amounts of typechecking time and memory. This for instance led to the recent memory issue #1577. Though, in that case nothing was marked abstract or opaque, so your changes might work well with Agda.

I have not measured it, but the overwhelming majority of users of these functions were already passing _ and allowing Agda to infer the values, so I'm disinclined to sweat it.

lowasser and others added 5 commits November 8, 2025 11:55
Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
…da.md

Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
…da.md

Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
…s.lagda.md

Co-authored-by: Fredrik Bakke <fredrbak@gmail.com>
@lowasser
Copy link
Collaborator Author

lowasser commented Nov 8, 2025

A clean build on my machine took 30 seconds less after this PR for some unclear reason, so I'm pretty confident this doesn't do anything disastrous.

@fredrik-bakke
Copy link
Collaborator

FYI we have the scripts/remove_unused_imports.py script to automate the import optimization. I went ahead and ran it on the files you touched.

@fredrik-bakke
Copy link
Collaborator

A clean build on my machine took 30 seconds less after this PR for some unclear reason, so I'm pretty confident this doesn't do anything disastrous.

That's amazing, great work!

@fredrik-bakke fredrik-bakke merged commit c67f94a into UniMath:master Nov 9, 2025
3 checks passed
fredrik-bakke pushed a commit that referenced this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants