proposed fix for a conversion inconsistency #434
Conversation
Currently the following happens. ``` gap> x:= JuliaEvalString( "2^60" ); 1152921504606846976 gap> GAPToJulia( 2^60 ); <Julia: 1152921504606846976> ``` The number `x` is not an immediate integer, and `Julia.Base.sqrt( x )` yields an error. The result of `GAPToJulia` (computed using `gap_to_julia`) looks correct. `JuliaEvalString` calls `gap_julia`, which calls `ObjInt_Int8` in this case; I propose to use `NewJuliaObj` instead. After this change, the tests exhibit a problem with the method ``` julia_to_gap(x::Int64) = x ``` which is simply wrong. Perhaps this proposal is not a good idea. Converting `Int64` to GAP becomes more expensive because the argument has to be checked. For the moment, I have no good idea to improve this.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #434 +/- ##
==========================================
- Coverage 73.51% 70.18% -3.34%
==========================================
Files 65 63 -2
Lines 5067 5074 +7
==========================================
- Hits 3725 3561 -164
- Misses 1342 1513 +171
🚀 New features to boost your workflow:
|
|
This reverts parts of PR #357. As such, I think it will just replace one set of problems with another; there is simply no good solution here. |
|
@fingolfin Thanks. In the case of I see that the current proposal causes efficiency problems, but which logical problems had been addressed by #357? (By the way, the test failure is due to an explicit test for GAP's opinion about the type of 2^60.) |
|
@ThomasBreuer see issue #332 |
2 participants
Currently the following happens.
The number
xis not an immediate integer, andJulia.Base.sqrt( x )yields an error.The result of
GAPToJulia(computed usinggap_to_julia) looks correct.JuliaEvalStringcallsgap_julia, which callsObjInt_Int8in this case;I propose to use
NewJuliaObjinstead.After this change, the tests exhibit a problem with the method
which is simply wrong.
Perhaps this proposal is not a good idea.
Converting
Int64to GAP becomes more expensive because the argument has to be checked.For the moment, I have no good idea to improve this.