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

Make “constraints” be GAP objects (separate from refiners, and such that a refiner points to a constraint) #21

Merged
merged 5 commits into from
Dec 3, 2021

Conversation

wilfwilson
Copy link
Collaborator

@wilfwilson wilfwilson commented Dec 1, 2021

This is the start of my work to address peal/vole#25 and peal/vole#26.

Anyway, this is roughly the direction that I'd like to go in:

  • Refiners are separate from constraints:

    • A constraint can be created separately from a refiner.
      • In particular, BacktrackKit, GraphBacktracking, and Vole will all share the same constraint objects.
    • A GAP-level refiner points to a constraint object as one of its members.
      • I'm not sure how much this will mess up how the Rust code in Vole handles GAP-level refiners...
    • There can be multiple refiners for the same constraint (for example, BacktrackKit, GraphBacktracking, and Vole each offer their own refiners for the constraint 'transporter of the set X to the set Y').
  • For now and maybe forever, refiners are the same as they were (i.e. records with various components), except that the members relating to the mathematical constraint (i.e. the property) are removed, and that information instead lives somewhere within the constraint object.

  • I want a user to be able to give a constraint object to a search functions without specifying a refiner for it. For some constraints, such as IsEven and IsOdd, there is no sensible refining that can be done anyway so it feels silly to create a dummy refiner that doesn't do anything.

    • But in general, this means that each package will need a way of choosing one (or more) refiners for each constraint. So e.g. we'll need some kind of notion of 'the default refiner for the set stabiliser constraint in Vole', and so on.

This PR will break GraphBacktracking and Vole. So we don't want to merge this until we are ready to update GraphBacktracking and Vole accordingly.

@wilfwilson wilfwilson added the enhancement New feature or request label Dec 1, 2021
@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #21 (22dc69f) into master (40d5eb3) will increase coverage by 0.21%.
The diff coverage is 85.26%.

❗ Current head 22dc69f differs from pull request most recent head f717dd9. Consider uploading reports for the commit f717dd9 to get more accurate results

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
+ Coverage   84.07%   84.28%   +0.21%     
==========================================
  Files          17       17              
  Lines        1356     1502     +146     
==========================================
+ Hits         1140     1266     +126     
- Misses        216      236      +20     
Impacted Files Coverage Δ
gap/constraints/conjugacyexample.g 68.75% <33.33%> (+1.28%) ⬆️
gap/constraints/normaliserexample.g 59.01% <33.33%> (+2.56%) ⬆️
gap/constraint.gi 80.30% <80.16%> (+0.30%) ⬆️
gap/BacktrackKit.gi 87.17% <100.00%> (-0.37%) ⬇️
gap/canonical.gi 96.55% <100.00%> (ø)
gap/constraint.gd 100.00% <100.00%> (ø)
gap/constraints/canonicalconstraints.g 100.00% <100.00%> (ø)
gap/constraints/graphconstraints.g 100.00% <100.00%> (ø)
gap/constraints/simpleconstraints.g 95.15% <100.00%> (-0.25%) ⬇️
... and 1 more

@wilfwilson wilfwilson force-pushed the constraint-objects branch 7 times, most recently from 637f2ca to c20451d Compare December 1, 2021 16:27
@wilfwilson wilfwilson marked this pull request as ready for review December 1, 2021 16:32
@wilfwilson wilfwilson force-pushed the constraint-objects branch 2 times, most recently from d34fe50 to d1602ff Compare December 3, 2021 15:56
@wilfwilson wilfwilson merged commit 939584e into peal:master Dec 3, 2021
@wilfwilson wilfwilson deleted the constraint-objects branch December 3, 2021 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant