cucapra / pollen

generating hardware accelerators for pangenomic graph queries
MIT License
24 stars 1 forks source link

Record Updates Allow Arbitrary Expressions, Update Pollen Tests #141

Closed susan-garry closed 9 months ago

susan-garry commented 9 months ago

The short change list:

  1. Update pollen examples to match new pollen syntax rules (such as putting everything in a function definition; more details below) and fix a logical bug in flip.pollen.
  2. In record updates, allow arbitrary expressions for the reference record (e.i., { e with f1:e1, f2:e2, ... } )
  3. ~Add automated testing via runt~ All automated tests pass after #140
  4. Rename the test folders to tests, for the sake of consistency with the calyx directory (I don't have a preference between one or the other, I just want to be consistent so that I don't have to think about which one I need depending on which directory I'm in)
  5. Rename the pollen extension to .pollen from .pol. .pol is much shorter, but apparently is already a file extension for "group policy settings." I'd be curious if anyone has feeling about .pol versus .pollen.

The longer explanation:

The majority of updates to the pollen examples consisted of

Additionally, I fixed a logical bug in flip.pollen related to our parset semantics - previously, it had been attempting to update a step's rank by iterating through rev(path.steps) and then emitting the step with it's orientation reversed without updating its index. However, as per our relational semantics, the order in which we emit steps does not matter, and we must instead specify its new index on path when we emit it, like so: { step with idx: max_step_idx - step.idx }.

What this pull request does not do is completely reconcile our pollen tests with the parser's semantics - paths.pollen refers to the steps in the path via path.steps[i], but the parser does not currently support array access. See #140

sampsyo commented 9 months ago

Excellent; all sounds great!

3. Add automated testing via runt

I see the new directory of tests, which seems great, but I don't set see a configuration file for Runt (or Turnt). Did this perhaps just not get checked in?

5. Rename the pollen extension to .pollen from .pol. .pol is much shorter, but apparently is already a file extension for "group policy settings." I'd be curious if anyone has feeling about .pol versus .pollen.

Sounds right to me! Less ambiguity => more better.

susan-garry commented 9 months ago

I see the new directory of tests, which seems great, but I don't set see a configuration file for Runt (or Turnt). Did this perhaps just not get checked in?

Ah, it looks like runt.toml was already merged into the main branch as part of #136 . I'll go ahead and update this PR to reflect that