Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation errors #2

Open
t0mpr1c3 opened this issue Dec 5, 2023 · 7 comments
Open

Validation errors #2

t0mpr1c3 opened this issue Dec 5, 2023 · 7 comments

Comments

@t0mpr1c3
Copy link
Contributor

t0mpr1c3 commented Dec 5, 2023

From the root directory of commit 33ce10f:

node k2f.mjs examples/pleat-tube/one-fourth.k one-fourth.f
node fnitout.mjs one-fourth.f

Gives the following output:

Will convert examples/pleat-tube/one-fourth.k (knitout) to one-fourth.f (formal knitout).
Gauge header indicates needles are 1 / 15 inches apart.
Will use carrier names (1 2 3 4 5 6 7 8 9 10) directly as yarn numbers.
Wrote to one-fourth.f

Reading 'one-fourth.f'...
Parsing 'one-fourth.f'...
Parsed 294 instructions.
Have 2 validation errors:
  Instruction 229: Expected yarn 1 at 22, but it is at 24.
    out + b.21 1 ; outhook 1
  Instruction 283: Expected yarn 2 at 22, but it is at 24.
    out + b.21 2 ; outhook 2
Trace contains 293 machine states.

Other files may fail to validate. This is the first one that I checked.

t0mpr1c3 added a commit to t0mpr1c3/fenced-tangle-supplemental that referenced this issue Dec 5, 2023
@ixchow
Copy link
Member

ixchow commented Dec 5, 2023

After a lunch conversation with @jlin98, we think there are two things going on here:

  1. there is a bug in k2f.mjs where it is using the attachment position of the carrier rather than the parking position of the carrier when writing the out instruction -- so any carrier that gets moved after its last operation would have the wrong out formal knitout instruction written. (Fixed in 847b2c3 on upstream and 451d3d2 in this repo.)
  2. it appears that both the translator and validator are inconsistent with the labeled tangles paper. The code here treats out's argument just like any other instruction and looks for the carrier before the needle mentioned; but the paper actually special-cases out and has it look for the carrier after the needle mentioned.

@t0mpr1c3
Copy link
Contributor Author

t0mpr1c3 commented Dec 5, 2023

2. it appears that both the translator and validator are inconsistent with the labeled tangles paper. The code here treats `out`'s argument just like any other instruction and looks for the carrier before the needle mentioned; but the paper actually special-cases `out` and has it look for the carrier _after_ the needle mentioned.

It looks to me like the validator in commit 33ce10f does make a special case for out when calculating the physical position.

@jlin98
Copy link
Contributor

jlin98 commented Dec 5, 2023

That's a mistake on my part. I actually caught the issue Jim mentioned about a week ago, but I only thought to fix it in the validator, and not in k2f.mjs. That resulted in the discrepancy.

@ixchow
Copy link
Member

ixchow commented Dec 5, 2023

Interesting. And our upstream formal-knitout repository (which I was using to check) didn't get that change. Interesting. I suppose we really need to resolve which behavior makes sense.

@ixchow
Copy link
Member

ixchow commented Dec 5, 2023

I am in support of breaking with the paper; that is, making the preconditions of all yarn-consuming operations the same. (Or, perhaps, implementing that break in the upstream formal-knitout repo and leaving the out-as-special-case behavior in this repository since it goes with the paper.)

Either way, we should update the README to reflect what's going on.

@t0mpr1c3
Copy link
Contributor Author

t0mpr1c3 commented Dec 5, 2023

I am in support of breaking with the paper; that is, making the preconditions of all yarn-consuming operations the same. (Or, perhaps, implementing that break in the upstream formal-knitout repo and leaving the out-as-special-case behavior in this repository since it goes with the paper.)

Is that a public repo? I would be glad to contribute there instead of here, if you would prefer to keep work in progress out of the repo linked to the paper.

@ixchow
Copy link
Member

ixchow commented Dec 5, 2023

Is that a public repo?

It is not, at least at the moment.

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

No branches or pull requests

3 participants