Skip to content

Incremental GA support starting from CSchankGA#4261

Closed
sarrasoussia wants to merge 89 commits intomainfrom
CSchankGA
Closed

Incremental GA support starting from CSchankGA#4261
sarrasoussia wants to merge 89 commits intomainfrom
CSchankGA

Conversation

@sarrasoussia
Copy link
Collaborator

@sarrasoussia sarrasoussia commented Jul 1, 2025

Work based on #4009

Sarra Soussia added 2 commits July 1, 2025 19:43
Copy link
Owner

@JacquesCarette JacquesCarette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a lot of this looks reasonable.

@JacquesCarette
Copy link
Owner

I am not sure the usual make test builds website, which is where this is happening.

@balacij
Copy link
Collaborator

balacij commented Aug 28, 2025

What commands are you running? If you can share logs, we can try to debug it.

@balacij
Copy link
Collaborator

balacij commented Aug 28, 2025

@JacquesCarette A basic make tests the website as well as the examples.

@balacij
Copy link
Collaborator

balacij commented Aug 28, 2025

Testing locally, I see errors, for example, with dblpend:

WARNING! Overwriting `cartesian` :: ConceptChunk
Symbol Table: dblpend: ERROR! Overwriting a chunk (`dblpend_multivector` :: `ConceptChunk`) with a chunk of a different type: `IdeaDict`
CallStack (from HasCallStack):
  error, called at lib/Database/Drasil/ChunkDB.hs:217:18 in drasil-database-0.1.1.0-AILWoHk0oaT2jusNLXYhMa:Database.Drasil.ChunkDB
make: *** [dblpend_gen] Error 1

This is by running make (with no arguments) in the code folder. Obviously, this is a new error which needs to be fixed.

@sarrasoussia With #4320, we changed the ChunkDB quite a bit. There is one hard rule now (immediately relevant to this error): chunks cannot share the same UID. In this case, you should be able to remove the IdeaDict in favour of keeping just the ConceptChunk version of dblpend_multivector.

@balacij
Copy link
Collaborator

balacij commented Aug 28, 2025

Can you please post the results of make --version?

Do you want to try debugging this over a Discord call tomorrow?

@sarrasoussia
Copy link
Collaborator Author

Can you please post the results of make --version?

Do you want to try debugging this over a Discord call tomorrow?

Yes please, thank youu

@sarrasoussia
Copy link
Collaborator Author

Note:

Right now we had both ClifKind (Scalar, Vector, etc.) and also the idea of picking grades with [Int]. That’s redundant — the kind already tells you the grade, and it doesn’t cover mixed cases (like scalar + bivector). So better to just drop ClifKind and use a list of Ints for grades.

Grades are just numbers between 0 and n (where n is the dimension). Anything outside that doesn’t make sense, so we need to handle it in code. There are a few ways:

  • ClifS Dimension [Int] Space This can allow ClifS (Fixed 3) [42] Real which is incorrect. So we need to implement a smart constructor that checks the list is in 0..n.
    mkClifS :: Dimension -> [Int] -> Space -> Maybe Space
    mkClifS (Fixed n) grades base =
    if all (\g -> g >= 0 && g <= n) grades
    then Just (ClifS (Fixed n) grades base)
    else Nothing

  • Wrap Int in a Grade newtype and provide a validator. Makes it clear when something is supposed to be a grade, not just any integer.
    newtype Grade = Grade Int
    deriving (Eq, Show)
    mkGrade :: Dimension -> Int -> Maybe Grade
    mkGrade (Fixed n) g =
    if g >= 0 && g <= n then Just (Grade g) else Nothing
    so now it is ClifS Dimension [Grade] Space

@JacquesCarette
Copy link
Owner

Your 'Note' is great. I would actually implement a combination of both, i.e.

data ClifS = ClifS Dimension Grades Space

and a smart constructor for Grades the way that you outline (but it would hide a [Int] not just Int) that the smart constructor for ClifS would call.

Rather than either of those returning Maybe, I would use error where you currently use Nothing.

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

Successfully merging this pull request may close these issues.

4 participants