RFC #66: Simulation time

[zyp]

Rendered.

[whitequark]

Something that I realized is that this class would also be really convenient for specifying frequencies of clocks.

amaranth.time?

[wanda-phi]

We have discussed this RFC on the 2024-06-10 weekly meeting. Several concerns were raised about the RFC:

• (@whitequark) while it is clear what time() should return within a process or testbench, the semantics of Simulator.time() is unclear while the simulator is not active; the issues are:
  • when stopped after advance(), should the returned time be the time of the last processed event, or the time of the next event to be processed (the latter of which can be undefined if there are no more pending events)?
  • (@wanda-phi) when stopped after run_until(), it is somewhat surprising that .time() can (and often will) return something different than the requested time point. It has been proposed to remove .time() from Simulator entirely, keeping it only on SimulatorContext, where it is well-defined.
• (@whitequark, @zyp) should we have reciprocal properties on Period to recover a frequency from it?
  • it is not well-defined when the period is zero; however, we can just raise ZeroDivisionError in that case

Additionally, a few bikeshed discussions have happened:

• (@whitequark) the naming of Simulator and SimulatorContext method that returns elapsed time; the proposed alternatives are: current_time(), next_time(), elapsed_time();
  • everyone seems reasonably happy with elapsed_time()?
• (@zyp) hertz, kilohertz, ... have been proposed for the frequency property accessors

[whitequark]

Denominating as the RFC hasn't been updated to address feedback in https://github.com/amaranth-lang/rfcs/pull/66#issuecomment-2159096137.

[mcclure]

Would it make sense to include a method for converting directly from Period to datetime.timedelta from the standard library, in case the user is working with some other library that uses those?

[whitequark]

Would it make sense to include a method for converting directly from Period to datetime.timedelta from the standard library, in case the user is working with some other library that uses those?

I've never heard of this class before or seen it used anywhere. Do you have a library in mind?

[zyp]

datetime.timedelta is actually decent prior art for what this RFC proposes; it serves a similar purpose and the set of supported operations is also pretty much identical. The difference is the lengths of the durations it's designed to work with.

Considering that datetime.timedelta only offers microsecond resolution and most clock periods tends to be an order of magnitude or two shorter, I don't think a conversion method would be very useful. Also, doing the conversion manually wouldn't be harder than datetime.timedelta(microseconds = round(period.microseconds)), which has the added benefit of being explicit about the precision loss.

[whitequark]

Proposal: instead of Period.kHz(1000) do Period(kHz=1000). This makes it very clear that it is a constructor and avoids any ambiguity. It's also the exact same number of characters.

[zyp]

I've had the same thought, so if you also think that's a good idea, I can revise the RFC accordingly. This raises a few questions though:

• Do we allow argumentless Period() to construct a zero-period? (I'm leaning yes.)
• Do we allow multiple arguments that add up? E.g. Period(s = 1, ms = 20) == Period(ms = 1020). (I'm leaning no, strong no for frequency arguments.)
• With properties and constructor arguments being separate namespaces, it'd be nice to keep them consistent (i.e. no kHz vs kilohertz), which raises the question of which to settle on? Alternatively we could do both as aliases.

[whitequark]

We have discussed this RFC on the 2024-07-09 weekly meeting. The disposition was to merge.

[whitequark]

Thanks!