<cr1901>
And so does pdm it seems, since I didn't need "allow-prereleases" to pull down a dev release
<cr1901>
if you want to depend on a dev version you can always use a git dependency <-- yes, this is what I am/have been doing. I just thought it would be nice to not need to dedicate "wasteful" commits to going from a git dep to pypi dep, cut a release, and then going from pypi dep back to git dep
<cr1901>
The scheme I described alleviates the need by "using the test index until the next release, and then using the real index thereafter (until I need a dev release again)"
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 8327d0a - Deploying to main from @ amaranth-lang/amaranth@625dac376ad6e59dd32b91b6f6384ccd6634f91f 🚀
mcc111[m] has quit [Quit: Idle timeout reached: 172800s]
<zyp[m]>
the sim, _ = sim_mod and _, m = sim_mod patterns looks unnecessary, in a «why make a fixture that needs to do two different things?» sense
cr1901_ has joined #amaranth-lang
<zyp[m]>
you could e.g. consider renaming the fixture sim, returning only the first element of the tuple and adding a method or property that lets you do m = sim.dut() or even m = sim.m
<zyp[m]>
and the mark.module stuff also looks a bit unergonomic, but I don't have any concrete suggestions for improvement
<whitequark[cis]>
adamgreig: I'm hitting an issue trying to use `residue` and `match_detected`
<whitequark[cis]>
>>> from amaranth.lib.crc.catalog import CRC32_ETHERNET
<whitequark[cis]>
>>> b = bytes.fromhex("6c6969696969e880887f701508004502006c08b900004011d3c80a000002540a3ff28aebee4e00585a5e00000000000181005bda9bcdd6df55bab5baebe035c0fdac4c47a878de12ae6d298ecb11bb53f083347dacacbd35dae9e9d9d5e6704a40505e7a9e4bef585576679da3341167f54fea5c6f8627683394811bdba1")
<cr1901>
zyp[m]: You're absolutely correct re: the sim_mod fixture. It _should_ be split. I'm worried about name collisions since "sim" and "mod" are so short fixture names, but maybe pytest has a way around that
<cr1901>
Re: the module and clk marks... I'll have to think about this. I think I have an idea
galibert[m] has joined #amaranth-lang
<galibert[m]>
No Choice?
<whitequark[cis]>
not in scope for 0.5 release
<galibert[m]>
Ok, gonna move from the Nice to have category I guess
<whitequark[cis]>
it would be nice to have
<whitequark[cis]>
however there are substantial unresolved questions
<whitequark[cis]>
and they haven't been getting resolved i guess
<galibert[m]>
Heh
<whitequark[cis]>
there's an internal version of Choice merged anyways, so there's no real pressure to have it in the public API. Amaranth itself can use it and everyone else can live without it
<galibert[m]>
Good enough
<galibert[m]>
Now that releases it’s not a problem to miss one
<galibert[m]>
* that releases are a thing it’s not
<_whitenotifier-6>
[amaranth] github-merge-queue[bot] created branch gh-readonly-queue/main/pr-1335-625dac376ad6e59dd32b91b6f6384ccd6634f91f - https://github.com/amaranth-lang/amaranth
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 8b43216 - Deploying to main from @ amaranth-lang/amaranth@f243cea0fbba486cab13bc6988bee8210db0db4a 🚀
<cr1901>
Is there an easy way to detect that a simulated module has clock domains, but add_clocks was never called? I want to error out in that case or at least warn (unless suppressed).
<Degi>
One can use a_BEL to assign an instance to a physical location, is there a similar thing for assigning signals etc. a physical wire?
<whitequark[cis]>
signals don't really ... exist by this point
<Wanda[cis]>
net routing is a much more complex thing than bel placement
<Wanda[cis]>
also it's ridiculously toolchain-dependent
<galibert[m]>
and I wish quartus explicit routing annotations upon noone
<whitequark[cis]>
I think the issue text describes it quite thoroughly
<jfng[m]>
i've used it a few times in formal testbenches
<mcc111[m]>
I actually have a vote on this one not listed in the options: Allow it but error if signed sign extension, specifically, is potentially going to happen
<zyp[m]>
I've never used it, so I wouldn't be opposed to option 1. If we keep it I think option 4 sounds the most reasonable, although I propose going even further and making operands with a shape other than unsigned(1) a hard error
<mcc111[m]>
jfng[m]: Did you use it with multi-bit registers?
<cr1901>
I have used the multi-bit form in Verilog code before, but I'm not particularly attached to it.
<tpw_rules>
zyp[m]: concur
<jfng[m]>
mcc111[m]: mostly with boolean logic, iirc
<jfng[m]>
i don't mind if it is removed
<cr1901>
option 4 seems best for me. Or option 1, seeing as most of the formal stuff has been pared back (and can be reintroduced later perhaps in a better form)
<Chips4MakersakaS>
I am not a user of it so will follow any outcome of discussion including removal.
<whitequark[cis]>
so obviously, m.d.comb += Assert(x.implies(y)) is the same as with m.If(x): m.d.comb += Assert(y), but using more obscure syntax
<zyp[m]>
the way I see it, I don't think a bitwise implies seems very useful, and if somebody intentionally want that, making them write out the expression makes it less confusing
<mcc111[m]>
it seems to me the most important thing is to avoid someone stumbling into an unexpected behavior. silent bool casting feels "surprising" potentially since none of our other logical operators do this
<mcc111[m]>
by contrast barring one configuration (e.g. erroring on signed implies) is surprising but won't lead to a surprising *behavior*, just having to add a cast (in the unlikely case it gets used)
<whitequark[cis]>
but it's also potentially surprising to have it not cast to bool
<whitequark[cis]>
as it's not actually obvious that x.implies(y) and x | y are the same kind of operation
<cr1901>
I've never used implies w/ signed ops in Verilog.
<cr1901>
Well, that's a limitation of Python for not providing extra infix ops
<whitequark[cis]>
I don't think anyone would ever use implies with signed operands
<galibert[m]>
most people, even with knowledge of formal logic, have a hard time to think of implies as a (a, b) -> c operator
<whitequark[cis]>
cr1901: this does not change one bit whether it's surprising or not
<jfng[m]>
`~p.bool() | q.bool()` is sufficiently explicit in intent, to not necessitate having a `.implies()` syntax sugar
<galibert[m]>
So i lean option 1
<cr1901>
I can write it out if I have to and make a function. I'd still prefer that to m.If(x): m.d.comb += Assert(y)
<mcc111[m]>
i personally like having these "verbose" logical builtins where the semantics are clear in the code
<mcc111[m]>
if people are concerned there might actually be ambiguity whether it's bitwise or logical, tho, i could see that as a valid argument for removal
<whitequark[cis]>
I think the If is more explicit and clear to someone who isn't accustomed to formal logic
<mcc111[m]>
the if is like… 3 or 4
<mcc111[m]>
lines of code tho. since you have to separtely set the 0
<zyp[m]>
I think it makes a lot of sense to remove it and teach the if-assert pattern
<whitequark[cis]>
the what?
<mcc111[m]>
s///, s/separtely/separately/
<zyp[m]>
and if you don't want an if, then you can also write out (~premise | conclusion)
<cr1901>
mcc111[m]: I think it's assumed that "y" is init to 0
<cr1901>
err 1, sorry
<jfng[m]>
whitequark[cis]: in a formal spec, readability is very important imo
<jfng[m]>
i agree that using 2-3 lines of code for an if-assert is a better choice than `.implies()`
<mcc111[m]>
so it seems like me and cr have weak preferences to keep in a multi-bit form, and pretty much everyone else is leaning remove. is that an accurate summary of the leanings of the room?
<Wanda[cis]>
I'm leaning remove
<cr1901>
mcc111[m]: I wanted implies() from the beginning, wq might have even added it for me loooong ago (don't remember). The formal stuff didn't really pan out, so I have a weak preference to keep.
<cr1901>
But not a hill I'm going to die on, even if I don't necessarily agree that m.If is clearer.
<whitequark[cis]>
I mean, a construct that's used everywhere is clearer to people who don't write formal testbenches than a formal-only construct; I don't see that as controversial or questionable
<whitequark[cis]>
whether we teach with m.If(x): m.d.comb += Assert(y), or m.d.comb += Assert(~x | y) (both of which can be written on one line, though the former will be usually written on two) is an open question, the deprecation notice will recommend the latter since that can be written inline as an expression
<whitequark[cis]>
thanks everyone who participated, I think the right choice here is to remove it, and potentially reintroduce later if there's strong consensus that it's useful
<whitequark[cis]>
this concludes the meeting
<mcc111[m]>
Are you actually going to mention Assert in the deprecation message?
<cr1901>
You think "m.If(x): m.d.comb += z.eq(y == 1)" is a common pattern?
<whitequark[cis]>
no, just say that x.implies(y) should be replaced with ~x | y, or possibly ~(x) | (y) (unsure)
<mcc111[m]>
whitequark[cis]: nod
<whitequark[cis]>
cr1901: I don't recall mentioning this pattern?
<cr1901>
It's how I parsed "a construct that's used everywhere is clearer". But you may have been just referring to m.If() by itself
<galibert[m]>
You'd need a m.Else(): m.d.comb += z.eq(1) to reimplment a full implies
<cr1901>
z(init=1)
<cr1901>
z = Signal(init=1)*?
<whitequark[cis]>
implies in a formal context is usually used for an assertion, where it's directly replaced with an If
<whitequark[cis]>
few people write expressions of the kind Assert(x.implies(y) | z.implies(t)), even fewer do it correctly, so I'm not considering that at all
<whitequark[cis]>
rephrasing for the benefit of others: I don't consider implies() an operator in its own right, practically speaking, but just a component of an Assert(x.implies(y)) expression (the overwhelmingly common use case of it)
* cr1901
nods
<galibert[m]>
If it's useful someday, it may be more comprehensible as Implies(x, y) which would be formal-only
<_whitenotifier-5>
[amaranth] github-merge-queue[bot] created branch gh-readonly-queue/main/pr-1336-f243cea0fbba486cab13bc6988bee8210db0db4a - https://github.com/amaranth-lang/amaranth
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 46b3158 - Deploying to main from @ amaranth-lang/amaranth@08aaac9a41eccde9cbb6df28573aa682a3c1ecc1 🚀
<Wanda[cis]>
I am... extremely confused by this code
<cr1901>
That is not a good sign lol
<cr1901>
Confused by which module/part? All of it?
<Wanda[cis]>
essentially yes
<Wanda[cis]>
like, I can untangle what it does if I spend enough time looking at it, but it's ... not pleasant
<cr1901>
Maybe I got overzealous with using pytest features.
<cr1901>
I _really_ don't like the notion of releasing a plugin only for myself, but this is the best amaranth test code I've written so far (in terms of sharing stuff between tests), so I think I'm gonna deploy anyway
<cr1901>
Sick of maintaining the same code in multiple places lol
<cr1901>
But noted that it's "not pleasant". That's not good
<zyp[m]>
I find the way fixtures work in pytest a bit weird
<zyp[m]>
partly convenient, partly weird
<cr1901>
I think I'm past the point where I think they're weird, and strictly in "convenience" territory lmao
<zyp[m]>
but then again my experience with pytest is only a few weeks old, and limited to some tests I wrote for C++ code…
<cr1901>
I've used it since last June, and my first introduction to fixtures was a proc-macro-based fixture crate in Rust in late 2022
<cr1901>
It's contrived (I mean, how much can you abstract a Blinky?!), but at least it gives me a base to work with.
<cr1901>
(You can get the current time elapsed from pysim, correct?)
<cr1901>
current sim time*
<zyp[m]>
that's one of the features that got descoped from #36
<cr1901>
If I parameterize clk periods (frequencies) in pytest, just for demonstration, I'd like to confirm that "yes, the simulator is ticking with the period I gave it".
<cr1901>
>but it needs a suitable return type to represent seconds with femtosecond resolution Hmm... femto is 10^-15. log2(10^15) is ~50 bits. So a 64-bit with scale factor 10^-15 would work
<zyp[m]>
I figure a SimulationTime could pretty much be a subclass of numbers.Rational with a .numerator that returns the raw femtosecond count and a .denominator that returns 10**15
<zyp[m]>
plus some thought into which operators makes sense to have and not, and which methods or properties could be convenient to add
<tpw_rules>
python is good at long integers, is integer number of femtoseconds not enough?
<zyp[m]>
do you want to manually deal in femtoseconds?
<zyp[m]>
currently clock intervals are given as «seconds as a float» and adding a SimulationTime type that also have a concept of what seconds are means that you can have implicit casts between them and use either