sim: Fix clock phase in add_clock having to be specified in ps.

[bl0x]

Now more in line with the documentation, which states that the process will wait phase seconds.

Ref:

amaranth/amaranth/sim/core.py

Lines 104 to 106 in c83b51d

	phase : None or float
	Clock phase. The process will wait ``phase`` seconds before the first clock transition.
	If not specified, defaults to ``period / 2``.

This was not true so far.
One had to give the clock phase wait time in ps.

[codecov-commenter]

Codecov Report

Merging #679 (e7233ff) into main (c83b51d) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   81.32%   81.34%   +0.01%     
==========================================
  Files          49       49              
  Lines        6464     6465       +1     
  Branches     1526     1526              
==========================================
+ Hits         5257     5259       +2     
  Misses       1015     1015              
+ Partials      192      191       -1     

Impacted Files	Coverage Δ
amaranth/sim/core.py	84.84% <100.00%> (+1.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c83b51d...e7233ff. Read the comment docs.

[whitequark]

cc @modwizcode

[bl0x]

Illustrative example:

from amaranth import *
from amaranth.sim import *

phase0_fail = Signal()
phase90_fail = Signal()
phase180_fail = Signal()
phase270_fail = Signal()
phase0_correct = Signal()
phase90_correct = Signal()
phase180_correct = Signal()
phase270_correct = Signal()

m = Module()
m.domains += ClockDomain("phase0_fail")
m.domains += ClockDomain("phase90_fail")
m.domains += ClockDomain("phase180_fail")
m.domains += ClockDomain("phase270_fail")
m.domains += ClockDomain("phase0_correct")
m.domains += ClockDomain("phase90_correct")
m.domains += ClockDomain("phase180_correct")
m.domains += ClockDomain("phase270_correct")

m.d.comb += [
        phase0_fail.eq(ClockSignal("phase0_fail")),
        phase90_fail.eq(ClockSignal("phase90_fail")),
        phase180_fail.eq(ClockSignal("phase180_fail")),
        phase270_fail.eq(ClockSignal("phase270_fail")),
        phase0_correct.eq(ClockSignal("phase0_correct")),
        phase90_correct.eq(ClockSignal("phase90_correct")),
        phase180_correct.eq(ClockSignal("phase180_correct")),
        phase270_correct.eq(ClockSignal("phase270_correct"))
        ]

sim = Simulator(m)
p = 1

# This is what is desired (?)
sim.add_clock(p, phase=0*p/4, domain="phase0_fail")
sim.add_clock(p, phase=1*p/4, domain="phase90_fail")
sim.add_clock(p, phase=2*p/4, domain="phase180_fail")
sim.add_clock(p, phase=3*p/4, domain="phase270_fail")

# This is what is needed (!)
sim.add_clock(p, phase=1e12 * (p/2 + 0*p/8), domain="phase0_correct")
sim.add_clock(p, phase=1e12 * (p/2 + 1*p/8), domain="phase90_correct")
sim.add_clock(p, phase=1e12 * (p/2 + 2*p/8), domain="phase180_correct")
sim.add_clock(p, phase=1e12 * (p/2 + 3*p/8), domain="phase270_correct")

def proc():
    for i in range(5):
        yield

sim.add_sync_process(proc, domain="phase0_fail")
with sim.write_vcd("bug_clock_phase.vcd"):
    sim.run_until(5)

[Lunaphied]

Thanks! Can you add this as an actual testcase as part of the PR for regression purposes?

[bl0x]

Where would that go? tests/test_sim.py?

[Lunaphied]

Yes; that's a good place to put it.

[bl0x]

On it.

[bl0x]

That should do it.

[Lunaphied]

Please make the suggested changes.

[bl0x]

@modwizcode Thanks for the review

[Lunaphied]

I think there was a bit of a misunderstanding, I clarified the style I intended, sorry about that.

[whitequark]

LGTM

[whitequark]

Thanks @bl0x and @modwizcode!