kscience/kmath
9
0
Fork 1
Code Issues Pull Requests 6 Packages Projects Releases 16 Wiki Activity

BigInt implementation #81

Merged
pklimai merged 3 commits from big-int into dev 2020-04-26 20:08:32 +03:00
Conversation 1 Commits 3 Files Changed 2 +1002
pklimai commented 2020-04-01 23:59:34 +03:00 (Migrated from github.com)
Copy Link
No description provided.
altavir (Migrated from github.com) requested changes 2020-04-09 18:53:49 +03:00
altavir (Migrated from github.com) left a comment
Copy Link

We need to discuss implementation details. Leaving a whole Int for a sign is a big waste. We can implement the whole BigInt as int array and use the first value for signum. In this case, we can use the inline class for BigInt.

But we need to be sure that the array is not accessible from the outside after creation.

We need to discuss implementation details. Leaving a whole Int for a sign is a big waste. We can implement the whole BigInt as int array and use the first value for signum. In this case, we can use the inline class for BigInt. But we need to be sure that the array is not accessible from the outside after creation.
kmath-core/src/commonMain/kotlin/scientifik/kmath/operations/KBigInteger.kt
altavir (Migrated from github.com) commented 2020-04-09 17:45:31 +03:00
Copy Link

Why use whole Int for simple three way flag? Could it be replaced by boolean? Enum?

Why use whole `Int` for simple three way flag? Could it be replaced by boolean? Enum?
altavir (Migrated from github.com) commented 2020-04-09 17:47:11 +03:00
Copy Link

Move to private constructor and move parameter checks to auxiliary builders.

Move to private constructor and move parameter checks to auxiliary builders.
altavir (Migrated from github.com) commented 2020-04-09 17:56:20 +03:00
Copy Link

Companion should be last declaration in the class

Companion should be last declaration in the class
altavir (Migrated from github.com) commented 2020-04-09 18:03:02 +03:00
Copy Link

Also all internal values must be marked as private

Also all internal values must be marked as private
altavir (Migrated from github.com) commented 2020-04-09 18:03:38 +03:00
Copy Link

should be virtual if it is not performance-critical

should be virtual if it is not performance-critical
altavir (Migrated from github.com) commented 2020-04-09 18:15:40 +03:00
Copy Link

It does not make sense to make it RingElement since all operations are already class members.

It does not make sense to make it RingElement since all operations are already class members.
altavir (Migrated from github.com) commented 2020-04-09 18:17:05 +03:00
Copy Link

Refactor to String.toBigInteger()

Refactor to String.toBigInteger()
pklimai (Migrated from github.com) reviewed 2020-04-15 20:37:05 +03:00
kmath-core/src/commonMain/kotlin/scientifik/kmath/operations/KBigInteger.kt
pklimai (Migrated from github.com) commented 2020-04-15 20:37:04 +03:00
Copy Link

Now using Byte for that

Now using Byte for that
pklimai commented 2020-04-15 20:37:49 +03:00 (Migrated from github.com)
Copy Link

Implemented changes that we discussed, please take a look.

Implemented changes that we discussed, please take a look.
Shimuuar (Migrated from github.com) reviewed 2020-04-15 22:25:06 +03:00
kmath-core/src/commonMain/kotlin/scientifik/kmath/operations/KBigInteger.kt
Shimuuar (Migrated from github.com) commented 2020-04-15 22:25:06 +03:00
Copy Link

Even byte is wasteful. Especially considering that my eat whole word due to alignment. Why not use 2-complement?

Even byte is wasteful. Especially considering that my eat whole word due to alignment. Why not use 2-complement?
pklimai (Migrated from github.com) reviewed 2020-04-15 22:42:14 +03:00
kmath-core/src/commonMain/kotlin/scientifik/kmath/operations/KBigInteger.kt
pklimai (Migrated from github.com) commented 2020-04-15 22:42:14 +03:00
Copy Link

This may be a good idea, but I suspect (performance) problems with 2-complement will appear when we try to operate with numbers of different length. At least the classical Java BigInt implementation stores magnitude and signum separately (http://developer.classpath.org/doc/java/math/BigInteger-source.html)

This may be a good idea, but I suspect (performance) problems with 2-complement will appear when we try to operate with numbers of different length. At least the classical Java BigInt implementation stores magnitude and signum separately (http://developer.classpath.org/doc/java/math/BigInteger-source.html)
altavir (Migrated from github.com) approved these changes 2020-04-26 20:08:24 +03:00
Sign in to join this conversation.
Reviewers
No reviewers
altavir
Labels
Clear labels   
bug

Something isn't working

  
dependencies

Pull requests that update a dependency file

  
discussion

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
feature

A new feature request

  
good first issue

The issue awaits its hero. Contributions are welcome

  
misc

Infrastructure tasks, cosmetic changes and miscellaneous tasks

  
performance

Performance optimization issue

  
question

Further information is requested

  
test

Add tests for some functionality

  
use case

A use case for library or feature

  
wontfix

This will not be worked on

No Label
bug
dependencies
discussion
documentation
duplicate
feature
good first issue
misc
performance
question
test
use case
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: kscience/kmath#81
No description provided.
Delete Branch "big-int"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?