Skip to content

Add Queue datatype - #3084

Draft
silas-hw wants to merge 79 commits into
agda:masterfrom
silas-hw:queue
Draft

Add Queue datatype#3084
silas-hw wants to merge 79 commits into
agda:masterfrom
silas-hw:queue

Conversation

@silas-hw

@silas-hw silas-hw commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Will eventually resolve #3083. Uses the two-list method mentioned in #3072 (but in a PR not generated by an LLM this time).

There may be some poor stylistic choices due to my unfamiliarity with the standard library, for which I am sorry! There are some 'low hanging' basic operations that could still be added, but I think those would be good for separate PRs.

@silas-hw
silas-hw marked this pull request as draft July 20, 2026 21:29
@silas-hw
silas-hw marked this pull request as ready for review July 20, 2026 21:32
@JacquesCarette

Copy link
Copy Markdown
Collaborator

When adding new functionality, we tend to make somewhat larger PRs than when fixing things. In particular, a bunch of properties should be proved too. The main reason is that quite often the proofs reveal when some of functionality was written in a correct-but-hard-to-use (or reason about) manner. So basically all of #3083 will need to be done in this PR.

On the other hand, asking for reviews of the intermediate steps is a good idea.

Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/TwoList/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
toList empty = []
toList (queue dq-hd dq-tail eq) = dq-hd ∷ (dq-tail ++ (reverse eq))

-- Create a Queue from a List, such that the elements

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably consider fromSnocList too.

@jamesmckinna jamesmckinna left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some alternative suggestions as to implementation.

Against @JacquesCarette , and because I'm a bit more 'OO-minded', I tend to put things like Empty/isEmpty as manifest fields of the record, because then I don't have to think about the scope management. Similarly toList, because all the pieces are at-hand without further ado... but YMMV.

As for references for this kind of implementation, probably Okasaki's "Purely functional data structures" would be the go-to citation?

Oh, and: fix-whitespace!

Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
@gallais

gallais commented Jul 21, 2026

Copy link
Copy Markdown
Member

As for references for this kind of implementation, probably Okasaki's "Purely functional data structures" would be the go-to citation?

There is also https://doi.org/10.1017/S0956796800001489

@jamesmckinna

Copy link
Copy Markdown
Collaborator

When adding new functionality, we tend to make somewhat larger PRs than when fixing things. In particular, a bunch of properties should be proved too. The main reason is that quite often the proofs reveal when some of functionality was written in a correct-but-hard-to-use (or reason about) manner. So basically all of #3083 will need to be done in this PR.

On the other hand, asking for reviews of the intermediate steps is a good idea.

So, eg.:

toList-fromList : toList (fromList xs) ≡ xs

The converse direction requires some simulation relation on Queues... but that's (probably) worth discussing/thinking about a bit more... although the toList observer of 'internal state' is already one kind of characteristic...

@silas-hw
silas-hw marked this pull request as draft July 21, 2026 14:03
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
Comment thread src/Data/Queue/Base.agda Outdated
silas-hw and others added 5 commits July 22, 2026 11:18
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
Co-authored-by: jamesmckinna <31931406+jamesmckinna@users.noreply.github.com>
@jamesmckinna

Copy link
Copy Markdown
Collaborator

hmmm... circular dependencies? Does splitting the Spec module into two help at all?

@silas-hw

silas-hw commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

I think it would rather be splitting Instances. I.e. have TwoList-RawQueue defined as an instance somewhere, have that imported into Properties, which can then be imported into some module that defines TwoList-IsQueue - but this does go against the current style.

Or maybe moving the definitions used in the instance of IsQueue to the Instances file itself, but this would result in a Instances file containing mostly properties.

Comment thread src/Data/Queue/TwoList/Base.agda Outdated
Comment thread src/Data/Queue/TwoList/Base.agda Outdated
Comment on lines +90 to +95
nullxs→xs≡[] : Null xs → xs ≡ []
nullxs→xs≡[] [] = refl

xs≡[]→nullxs : xs ≡ [] → Null xs
xs≡[]→nullxs xs≡[] rewrite xs≡[] = []

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can these be refactored to make use of the lemmas added in #3091

_ : False (empty? q)
_ rewrite eq = _

-- IsQueue bundles RawQueue with proofs of a Queues correctness,

@jamesmckinna jamesmckinna Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The operative word here is 'bundle'.

I think that IsQueue, even if only for consistency with existing distinctions between 'structure' and 'bundle' (eg. see #3052), should be considered as a structure, and thus parametrised on Q and the rawQ : RawQueue Q .

A bundled Queue would then be a record with three fields:

  • the type constructor Q
  • its associated rawQ
  • the proof that it, indeed, satisfies IsQueue

Cf. #2252

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh, and a propos @JacquesCarette 's jeremiad in #3052 we should perhaps consider instead write QueueSig rather than RawQueue...?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see! And from the perspective of a relatively new developer, XSig is more immediately obvious than RawQueue, so I'm in favor of that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And for the bundle, how about 'lifting' the Sig functions to be on the bundle itself?, e.g.

record Queue (A : Set a) : Set (suc a) where
  field
    Q : Set a  Set a
    q : Q A
    rawQueue : RawQueue A Q
    isQueue : IsQueue A rawQueue

open Queue

-- 'lifted' functions
enqueue  : {a : Level}  {A : Set a}  A  Queue A  Queue A
enqueue x q' = record { Q = q' .Q ; q = RawQueue.enqueue (q' .rawQueue) x (q' .q) ; rawQueue = q' .rawQueue ; isQueue = q' .isQueue }

Is there some nice automatic way of doing this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ooh. Maybe I was too hasty/casual in my suggestion above regarding the bundle (I think the structure story makes sense)...

Is it obviously a good idea to allow the basic operations potentially to change the underlying type constructor Q upon each invocation? (your suggested lifted versions obviously do not, but it isn't ruled out for the implied API defined by the bundled version?)

Perhaps some more thought required?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Queue datatype

6 participants