Open singularitti opened 8 months ago
I think this is a very reasonable suggestion. If other folks agree (e.g. @mfherbst, @cortner, @jgreener64), maybe you could make a PR to this effect?
I don't have a strong view. StructEquality looks useful but I have never used it myself so can't really comment how practical it is. I'm happy for this to go ahead .
Hi @rkurchin, sure. I could make a PR to test this change.
Sounds like a good quick solution to get a solid behaviour.
One edge case I can think of where one should think a second of the desired behaviour is the case where two atoms/systems only differ by the extra attributes, which are stored, e.g. one atom might store some extra information (e.g. a tag or a property relevant only to an implementing library), which from the AtomsBase point of view is not relevant. Should that then compare equal ? The safe choice is probably no, but I can see cases where one might argue yes as well. Anyone has strong feelings about this?
Sounds okay to me.
I would err on the side of safety, I would be surprised if two things were equal when something was different. The user shouldn't have to worry too much about which bits AtomsBase cares about.
@mfherbst Good point. We will store training data in structures. Not sure how else to do this. Can there be a data dict which is excluded in the == ?
Btw for testing equality in tests there is also the AtomsBaseTesting package with some utilities.
Could also ≈ be added in a reasonable way?
StructEquality.jl provides @struct_isapprox
, so I think yes?
I'd love to have this PR, I'm current hand-writing equality and ≈ tests.
Sorry I was pretty busy before, but recently I think I will have time to do this.
I was adding support to this package in my code, where in the future, the conversions between
FlexibleSystem
and myCell
types used inCrystallographyBase.jl
andCrystallography.jl
is possible.However, when I was testing this conversion, I found that
Atom
&FlexibleSystem
contains mutable types likeDict
, therefore their==
are not automaticallytrue
since:Which caused comparisons between these types feeling wierd:
As you can see, their fields are equal, but they are not equal.
This might be surprising when you want to compare them, and this is what I encountered earlier: I have to write more code to do simple tests.
One solution is to use
StructEquality.@struct_hash_equal_isequal
:Then the above tests will all be
true
s.In fact, I use
@struct_hash_equal_isequal
in all my packages:Similar packages are: