JacquesCarette
GamePhysics: Move `defSymbols` to TermMap to avoid duplicates
Contributes to #4036
After making this change I realized that it might only hurt with resolving #4126; maybe @balacij could take a look to see if this is a good change or not? I am not sure what is more of a priority at this point, having no duplicates within chunk tables or no duplicates across all chunk tables.
This removes duplicate ConceptChunks (within the ConceptChunks table)? Where do the duplicates come from?
I think @jackwyand already searched through the problematic "within same table, chunks with shared UIDs but different data" (i.e., major issues). So, I would say that the across table issues would be more important right now because across table issues are hard issues for #2873 while within table ones are soft issues.
If the within table that are major issues are under control, that is indeed different!
To confirm, the duplicates within the concept table that this was fixing were UIDs of the constraints added in defSymbols conflicting with the physics concepts also added in the concept table. However, if the between conflicts are more important than maybe this should be closed?
Can you give me an explicit list of the chunks that were conflicting? Was it the same chunks but add to two tables or something else?
The problematic chunks are the following:
-
angularVelocity -
force -
gravitationalConst -
momentOfInertia -
orientation -
position -
restitutionCoef -
torque -
velocity
The output states that they are all inserted twice in the ConceptMap (which is where the conceptChunks list is added to). These all come from the inputConstraints that are added to the defSymbols list, since they are all physics constraints which share a UID with the corresponding physics concepts in physicCon, also added in the ConceptMap.
To answer your question, it is not the same list being added twice, but the concepts and corresponding constraints sharing a UID.
Can you detail out how this happens with one of them, like say force? I'm not 100% sure that I understand your explanation, seeing the details would help me be sure that I do.
Sure!
In the conceptChunk list that is added to the ChunkDB, there is a list called defSymbols (Ln 149)
https://github.com/JacquesCarette/Drasil/blob/def8f55cbdd4cd2b2cd857e8b6958009b4513c06/code/drasil-example/gamephysics/lib/Drasil/GamePhysics/Body.hs#L144-L153
Within this list, there are two other lists added, including inputConstraints:
https://github.com/JacquesCarette/Drasil/blob/def8f55cbdd4cd2b2cd857e8b6958009b4513c06/code/drasil-example/gamephysics/lib/Drasil/GamePhysics/Unitals.hs#L26-L27
forceCons, added in inputConstraints:
https://github.com/JacquesCarette/Drasil/blob/def8f55cbdd4cd2b2cd857e8b6958009b4513c06/code/drasil-example/gamephysics/lib/Drasil/GamePhysics/Unitals.hs#L317-L320
shares a UID with force since it uses that chunk in its creation
https://github.com/JacquesCarette/Drasil/blob/def8f55cbdd4cd2b2cd857e8b6958009b4513c06/code/drasil-example/gamephysics/lib/Drasil/GamePhysics/Unitals.hs#L334
force is then also added in physicCon, which contains all of the general physics concepts. physicCon is evidently also added in conceptChunks, causing the duplicate warning.
Hopefully this makes sense!
Yes, it does.
I think the real problem here is that forceCons has the same UID as force. These are separate things. Unless force in GamePhysics is only ever "force of gravity"?
Feels to me that constrained' is a bad combinator.
@jackwyand and I spoke about this in person. We agreed to delay working on this one a bit because it is not entirely essential to #2873. It is definitely something we want to tackle soon, however.
@JacquesCarette's comment here:
I think the real problem here is that forceCons has the same UID as force. These are separate things. Unless force in GamePhysics is only ever "force of gravity"?
Looks like the 'right issue to tackle' first.
Aside: "force" in this context is "net force" -- nothing to do with gravity.
We might need to figure out how the information in UncertQ might need to be reorganized, and possibly be merged back into D-Q-D or elsewhere. As it is right now, UncertQ appears to be structured after the columns shown in the two kinds of data constraints tables we have in the SRS (inputs and properties of a correct solution), so the "knowledge" here probably needs to be reorganized somehow.
If we don't want to change how the knowledge here is organized/placed, then we just need to change the UID and ensure that UncertQs are registered in the ChunkDB.
One key piece of information that @JacquesCarette mentioned about the "reasonable values" that the UncertQs associate with D-Q-Ds: "think about the order of magnitude of the numbers rather than the specific numbers." For example, in the context of a beam, a length of 50m is sane, a similar length 2 orders magnitude up or down is also fine. But a "beam" of length 1E-100m is not.