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

Add tests of kmath-memory and make all the memory operations on all the platforms use little-endian byte order #163

Closed
CommanderTvis wants to merge 4 commits from unify-endianness into dev
pull from: unify-endianness
merge into: kscience:dev
Conversation 9 Commits 4 Files Changed 5 +108 -51
CommanderTvis commented 2020-12-10 14:16:04 +03:00 (Migrated from github.com)
Copy Link
No description provided.
altavir (Migrated from github.com) requested changes 2021-01-05 17:01:15 +03:00
altavir (Migrated from github.com) left a comment
Copy Link

Keeping endians across platform does not make any sense. It is possible to define additional memory, that is shareable between platforms, but I do not want to guarantee that for the default memory.

Keeping endians across platform does not make any sense. It is possible to define additional memory, that is shareable between platforms, but I do not want to guarantee that for the default memory.
CommanderTvis commented 2021-01-05 17:03:13 +03:00 (Migrated from github.com)
Copy Link

Keeping endians across platform does not make any sense. It is possible to define additional memory, that is shareable between platforms, but I do not want to guarantee that for the default memory.

It does. We provide ways to wrap existing object as memory (Memory.wrap), and it has to work equally on all the supported platforms.

> Keeping endians across platform does not make any sense. It is possible to define additional memory, that is shareable between platforms, but I do not want to guarantee that for the default memory. It does. We provide ways to wrap existing object as memory (Memory.wrap), and it has to work equally on all the supported platforms.
CommanderTvis commented 2021-01-05 17:03:53 +03:00 (Migrated from github.com)
Copy Link

e.g., Memory.wrap(byteArrayOf(0,0,0,1)).read { readInt(0) } must be persistent

e.g., `Memory.wrap(byteArrayOf(0,0,0,1)).read { readInt(0) }` must be persistent
altavir commented 2021-01-05 17:36:42 +03:00 (Migrated from github.com)
Copy Link

I can't see, why it should. We do not allow to access the inner layout of memory, only the offset.

I can't see, why it should. We do not allow to access the inner layout of memory, only the offset.
CommanderTvis commented 2021-01-05 17:39:16 +03:00 (Migrated from github.com)
Copy Link

I can't see, why it should. We do not allow to access the inner layout of memory, only the offset.

Because it's almost useless it is does not because each platform must receive separate byte arrays.

> I can't see, why it should. We do not allow to access the inner layout of memory, only the offset. Because it's almost useless it is does not because each platform must receive separate byte arrays.
CommanderTvis commented 2021-01-05 17:39:48 +03:00 (Migrated from github.com)
Copy Link

Why do we need wrapping of array if it works in absolutely different way on each platform

Why do we need wrapping of array if it works in absolutely different way on each platform
altavir commented 2021-01-05 18:22:45 +03:00 (Migrated from github.com)
Copy Link

Currently, Memory is the inner implementation detail. Do we even need wrapping of arrays?

Currently, Memory is the inner implementation detail. Do we even need wrapping of arrays?
CommanderTvis commented 2021-01-05 18:35:57 +03:00 (Migrated from github.com)
Copy Link

We may drop it as well as file reading in JVM

We may drop it as well as file reading in JVM
altavir commented 2021-01-05 19:30:05 +03:00 (Migrated from github.com)
Copy Link

File reading does not break any rules, but I agree it is useless right now, so it could be commented out.

File reading does not break any rules, but I agree it is useless right now, so it could be commented out.
CommanderTvis commented 2021-04-27 18:51:11 +03:00 (Migrated from github.com)
Copy Link

Stale PR, the problem itself has to be discussed

Stale PR, the problem itself has to be discussed

Pull request closed

This pull request cannot be reopened because the branch was deleted.
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
0.2
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#163
No description provided.
Delete Branch "unify-endianness"

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?