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

drasil-lang: clean up UID-related stuff and move it away from the other modules #4016

Merged
merged 4 commits into from
Mar 2, 2025

Conversation

balacij
Copy link
Collaborator

@balacij balacij commented Feb 28, 2025

Contributes to #2885

Once #2873 is merged, UID will move there, so I'm at least moving the module to a different namespace now, locally in drasil-lang.

@balacij balacij marked this pull request as ready for review February 28, 2025 22:07
@JacquesCarette
Copy link
Owner

I don't quite understand: #2873 is not a PR but an issue? Is there a PR that this is waiting on?

theory:v_x2 -> p_x2;
theory:v_y2 -> velocity;
theory:v_y2 -> p_y2;
v_x1:theory -> velocity;
Copy link
Owner

Choose a reason for hiding this comment

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

in this case, theory:v_x1 and v_x1:theory both make little sense... curious where it comes from. The reasoning above is sound, so it's super weird that the theory now comes at the end!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That UID is for:

velXGD_1 :: GenDefn
velXGD_1 = gdNoRefs (equationalModel' velXQD_1) (getUnit velocity) (Just velXDeriv_1) "velocityX1" [{-Notes-}]

Relevant 'backend' code:

-- | Smart constructor for 'EquationalModel's, deriving UID+Term from the 'QDefinition'
equationalModel' :: QDefinition e -> ModelKind e
equationalModel' qd = MK (EquationalModel qd) (modelNs $ qd ^. uid) (qd ^. term)

modelNs :: UID -> UID
modelNs = nsUid "theory"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i.e., theory is a namespace

Copy link
Owner

Choose a reason for hiding this comment

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

So you want namespaces to appear on the right?

Copy link
Collaborator Author

@balacij balacij Mar 2, 2025

Choose a reason for hiding this comment

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

If we have nest C $ nest B $ nest A X, the UID before this PR ends up as: C:B:A:X.

I read the relationships between C, B, and A as $C \subset B \subset A$. The code would then be read as "nest $X$ in $A$, then create a new namespace for $X$ within $A$, etc.". In which case, the update would make more sense because it shows $X$ near the smallest namespace, C (e.g., X:C:B:A).

Thinking about it again... maybe I was confused? The documentation is unclear about the relationship between the nested namespaces. I'm not sure, and I'm impartial because I'm fairly convinced we're going to have to completely rework the UID system once we get to the real work on editing the chunk graph, theory refinements, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or did you mean that you agreed with my view and just wanted the larger namespaces to appear leftmost rather than rightmost? e.g., A:B:C:X.

Copy link
Owner

Choose a reason for hiding this comment

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

I want our name-spacing conventions to be the same as Haskell's and Agda's, so Data.List.Membership for example. I don't see this as having anything to do with subsets, but rather is all about hierarchical classification.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that does make more sense. I just updated that now in 361b507 and e2f687c .

@balacij
Copy link
Collaborator Author

balacij commented Mar 1, 2025

Sorry, I mean "once the prototype code written for #2873 is merged, UID will live in drasil-database much closer to the upgraded ChunkDB".

@JacquesCarette JacquesCarette merged commit e1790c7 into main Mar 2, 2025
5 checks passed
@JacquesCarette JacquesCarette deleted the uidClean branch March 2, 2025 22:39
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.

2 participants