<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 5b93140 - Deploying to main from @ amaranth-lang/amaranth@055d6a4ec80e2c8754941222673968627dd1ffe9 🚀
<whitequark[cis]>
the error message is almost certainly right about what the problem is
<_whitenotifier-3>
[amaranth] whitequark closed pull request #1457: sim: make driving parts of a signal from distinct modules possible. [0.5 backport] - https://github.com/amaranth-lang/amaranth/pull/1457
<mcc111[m]>
since i can't find a place toplevel.clk is written to and i don't know the exact definition of sim.add_clock (as those docs are still pending ofc) i'm unsure how to proceed
<Wanda[cis]>
sim.add_clock drives a clock domain's clk signal
<Wanda[cis]>
and since it is already driven (from sync.clk.eq(self.clk), that's a driver conflict
<Wanda[cis]>
the fact that self.clk is, in turn, undriven is irrelevant
<whitequark[cis]>
I've documented the entirety of the simulator
<mcc111[m]>
Wanda[cis]: why does it drive sync.clk and not self.clk?
<mcc111[m]>
whitequark[cis]: Fantastic, thanks
<whitequark[cis]>
it drives ClockSignal("sync")
<whitequark[cis]>
it has no idea that self.clk even exists (how would it know?)
<mcc111[m]>
.. okay, the answer to my question appears to be "because it's documented as acting on the domain"
<Wanda[cis]>
because it doesn't make an effort to attempt to reverse assignment chains to find that there's an undriven upstream signal
<mcc111[m]>
Catherine: can you explain what the `self.boot` domain in this code is doing? (I think you might have written that first block of lines)
<Wanda[cis]>
anyway, the simplest way to resolve your problem is to just make a process that does await ctx.delay(half clock period) and ctx.set(clk, ...) in a loop
<mcc111[m]>
Wanda[cis]: that is simple
<Wanda[cis]>
you could also wrap the toplevel in another module, create a clock domain in it, drive clk from its clock, and then add_clock on that domain in turn, but that's a lot of trouble to go through, and add_clock is not complicated enough to warrant this
<mcc111[m]>
thanks y'all
<whitequark[cis]>
mcc111: it was replicating some verilog thing i got as a reference
<zyp[m]>
I also have another comment; last week Wanda commented that «I'm not convinced the `*hertz` accessors should return a Fraction instead of a float», and I replied that the main argument is that consistency is good
<zyp[m]>
I've been thinking and I'm not convinced anymore that anything returning a Fraction is actually useful
bai has joined #amaranth-lang
<zyp[m]>
so we could consider switching everything that returns Fraction to float
<zyp[m]>
I figure loss of precision only really matters when you want to look at a small difference between two large numbers, and given that Period has operators, the conversion to float will happen after the subtraction
<whitequark[cis]>
yes, if you're just using floats for storage, you're never going to be worse than 2^-24 or so off
<whitequark[cis]>
which is 60 ppm
<whitequark[cis]>
* which is 60 ppb
<Wanda[cis]>
2^-53, these are doubles
<zyp[m]>
what I had in mind was a testbench doing something like a = ctx.elapsed_time(); await something(); b = ctx.elapsed_time(); foo(b - a)
<zyp[m]>
2^53 femtoseconds are roughly 9 seconds, so floats for periods above 9 seconds won't have femtosecond precision
<whitequark[cis]>
yes. so if you're using them for storage (float(b - a)) you're fine, for calculations (float(b) - float(a)) you're not
<whitequark[cis]>
actually, wouldn't it only be *hertz accessors that return fractions?
<zyp[m]>
no, .picoseconds would be something like return Fraction(self.femtoseconds, 1000)
<whitequark[cis]>
would that ever result in unwanted loss of precision though?
<zyp[m]>
anyway, fractions were my idea, and I've changed my mind, so I suggest that unless somebody else has a compelling reason to keep them, we switch them all to float
<whitequark[cis]>
I'm happy to go for float
<Chips4MakersakaS>
Agree; femtoseconds can always be used for exact value.
<Wanda[cis]>
so, the RFC doesn't specify what happens when you print (__str__ or __format__) a Period
<Wanda[cis]>
this sounds like something that could be worth specifying
<Wanda[cis]>
perhaps along with several format specifiers
<Wanda[cis]>
for "print as nanoseconds", etc
<zyp[m]>
ah, yeah, that sounds nice
<whitequark[cis]>
I propose "print as the biggest SI unit", e.g. "1s", "1001us", "100000001ns" etc
<zyp[m]>
that sounds reasonable to me
<Wanda[cis]>
whitequark[cis]: I don't follow? "print as the biggest SI unit capable of representing exactly"?
<zyp[m]>
that's how I interpreted it
<Wanda[cis]>
that's... rather strange
<Wanda[cis]>
esp. since "1.000000001s" is likewise exact and easier to read
<whitequark[cis]>
it's not
<whitequark[cis]>
I have to count 0's to find out what the factor is, instead of looking at the ns
<whitequark[cis]>
s/ns/unit/
<zyp[m]>
which is easier depends whether you care about the upper or the lower end, really
<zyp[m]>
I think as long as we have a __format__ that allows specifying the unit, I think it's bikeshed what the default it, whether it's the largest unit that allows an exact integer or the smallest unit that has a nonzero number left of the decimal point
<zyp[m]>
s//`/, s//`/, s/it/is/
<jfng[m]>
can we provide a helper where the format is configurable (e.g. for timestamps, you want the same unit everytime), in addition to `__str__` ?
<whitequark[cis]>
we're not going to see a lot of large Period values that are additive: pretty much the only place is elapsed_time
<whitequark[cis]>
and yes, that's what __format__ can do
<zyp[m]>
jfng[m]: that's what `__format__` is
<jfng[m]>
ah, ok
<zyp[m]>
whitequark[cis]: I think the issue is something like `Period(megahertz = 30)` which would then print as `33333333fs`
<zyp[m]>
33.333333ns seems more readable to me
<whitequark[cis]>
I think given all this we should actually leave the formatting unspecified
<zyp[m]>
I can put it in the future section
<tpw_rules>
maybe format it as a Fraction? :3
<whitequark[cis]>
that doesn't seem useful
<whitequark[cis]>
I mean formatting it as a fraction
<whitequark[cis]>
it's like the worst of all options?
d_olex has joined #amaranth-lang
<zyp[m]>
fraction reprs are hard to read, and that's a good argument for not having a default that'll result in stuff like 33333333fs :)
<tpw_rules>
it was mostly in jest, i agree with leaving it unspecified for now.
<zyp[m]>
let's do the unresolved questions
<zyp[m]>
first, where do we put Period?
<Wanda[cis]>
amaranth.lib.time?
<whitequark[cis]>
I was always thinking that amaranth.lib is for gateware
<Wanda[cis]>
lib.data is not gateware
<zyp[m]>
how about amaranth.utils?
<whitequark[cis]>
amaranth.utils should not exist
<Wanda[cis]>
agreed; if anything, if we don't find another place for it, it can go to amaranth.time
<Wanda[cis]>
which was going to be my second pick
<whitequark[cis]>
it does, which is regretful, but it should not
<jfng[m]>
if non-stdlib parts of amaranth are going to depend on it, i don't think it should go in `amaranth.lib`
<whitequark[cis]>
yeah
<zyp[m]>
whitequark[cis]: should we have something like `lib` but for various non-gateware things?
<whitequark[cis]>
I was thinking of that
<whitequark[cis]>
dunno what to call it though
<zyp[m]>
amaranth.util.time?
<whitequark[cis]>
that's just utils all over again
<jfng[m]>
we can figure it out later, and put it in `amaranth.time` for now ?
<whitequark[cis]>
it should be banned as a word in projects
<whitequark[cis]>
jfng[m]: well I don't think we can since it's a public interface
<whitequark[cis]>
also, we have other unresolved questions
<whitequark[cis]>
> Should we disallow passing a negative frequency or accessing a frequency property on a negative period?
nyanotech has quit [Remote host closed the connection]
<zyp[m]>
whitequark[cis]: I'm leaning yes, negative frequencies seems nonsensical and it'll give a diagnostic when somebody makes one by accident
<whitequark[cis]>
yeah
<Wanda[cis]>
yeah
<Chips4MakersakaS>
About place. Should we consider Period part of core lang as Signal is?
<zyp[m]>
you mean put it in the prelude? I don't think it should be
<Chips4MakersakaS>
Neg. Freq also nonsensical to me
<zyp[m]>
I figure it'll mostly be either simulation or platform code using Period, which can explicitly import it
<whitequark[cis]>
it could be in hdl I suppose
<whitequark[cis]>
(not in the prelude)
<Wanda[cis]>
weird but hm
<Wanda[cis]>
no real objection
<whitequark[cis]>
yeah I don't object
<zyp[m]>
amaranth.hdl.time then?
<whitequark[cis]>
we don't have hdl submodules
<zyp[m]>
ah
<zyp[m]>
fair
<whitequark[cis]>
I think amaranth.hdl could be a nicer choice than amaranth.time because the hdl name is itself a good namespace
<zyp[m]>
so, from amaranth.hdl import Period would be the recommended way of accessing it
<whitequark[cis]>
whereas time clashes with the Python standard time
<whitequark[cis]>
zyp[m]: yes, although there is no harm in re-exporting it from `amaranth.sim`
<zyp[m]>
agreed, we should do so
<whitequark[cis]>
we ran out of time for this meeting. we can continue discussing this though
<Wanda[cis]>
is there a PoC already btw?
<zyp[m]>
I haven't written one
<Wanda[cis]>
mhm
<Wanda[cis]>
I may write one if I get sufficiently annoyed at xilinx reversing
<zyp[m]>
can we do the argument and property name bikeshed now?
<whitequark[cis]>
yeah
<whitequark[cis]>
for properties, there's no reason to accept the shortened form that I can think of: it has the wrong capitalization and isn't commonly used
<tpw_rules>
agreed
<Chips4MakersakaS>
i'm going afk but dont have strong feeling so anything pk for me.
<zyp[m]>
why does capitalization matter more for properties than named arguments?
<Chips4MakersakaS>
*ok
<_whitenotifier-3>
[amaranth] github-merge-queue[bot] created branch gh-readonly-queue/main/pr-1458-055d6a4ec80e2c8754941222673968627dd1ffe9 - https://github.com/amaranth-lang/amaranth
<whitequark[cis]>
named arguments look like syntax in this context (see also m.If), properties are just properties
<whitequark[cis]>
if Python had custom literals like C++, these would be the way to go for us and such
<whitequark[cis]>
this is actually what CXXRTL does
<whitequark[cis]>
without custom literals, we borrow Period() from Python proper and designate it Amaranth syntax
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 1d797d5 - Deploying to main from @ amaranth-lang/amaranth@bf98136e2101d62aabfe2d1274bde87bb9dcd497 🚀
<whitequark[cis]>
I guess it could be specified as just a list
<Wanda[cis]>
I don't like adding more moving pieces to the language for no good reason
<whitequark[cis]>
I agree with Wanda
<zyp[m]>
fair enough, so short arguments, long properties then?
<whitequark[cis]>
yeah
<whitequark[cis]>
then we can have each constructor be a @typing.overload
<whitequark[cis]>
this works well with typecheckers and such
<zyp[m]>
okay, I think we've answered enough that we could put it to a vote next week
<zyp[m]>
and I'll think about __format__ a bit too and see if I can't get that squeezed in
<whitequark[cis]>
sounds good
<whitequark[cis]>
thank you!
SpaceCoaster has quit [Read error: Connection reset by peer]
SpaceCoaster_ has joined #amaranth-lang
SpaceCoaster_ is now known as SpaceCoaster
FFY00 has quit [Ping timeout: 276 seconds]
SpaceCoaster has quit [Ping timeout: 260 seconds]
SpaceCoaster has joined #amaranth-lang
<zyp[m]>
<Wanda[cis]> "is there a PoC already btw?" <- I started writing one so I can prototype `__format__`
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] whitequark 4d513e3 - Deploying to main from @ amaranth-lang/amaranth@c78806f193d19b212fb5554038c88b6a94516b81 🚀
<whitequark[cis]>
Amaranth 0.5.1 has been released! everyone is encouraged to upgrade as there are numerous bug fixes included, as well as an important enhancement to the simulator that makes development of I/O cores much easier
<_whitenotifier-3>
[amaranth-lang/amaranth] whitequark deleted tag v0.5.1
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] whitequark cd0a090 - Deploying to main from @ amaranth-lang/amaranth@212cb2c57c191ef7442a9cb5ded39b6a07278ccd 🚀