[go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed index error of np.zeros #105

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

kimikitae
Copy link
Contributor

i found this error when i use zns-tools.nvme on zns
at this time
zone size :128MB
nr zone: 128

then index error was issued

finaly i fixed it

(its my first pull request, my pr is not good, im ready for any feedback)

@Krien
Copy link
Member
Krien commented Aug 28, 2024

Hi @kimikitae, thank you for your PR!
What index error did you encounter specifically? Was it an out of bounds error?
I will have a look soon to see if it works on multiple zone sizes, nr zones.

@kimikitae kimikitae closed this Aug 28, 2024
@kimikitae
Copy link
Contributor Author
kimikitae commented Aug 28, 2024

Hello ~!! @Krien thank you for your reply, but my pr may be wrong. Feedback is always welcome.
(I accidentally closed the pr and reopened it by clicking on it. don't worry)

in zns-tools.nvme/plot.py
in line 75, 76, 143, 188, 253, 254

i fixed code like this :
[-1, -1 - val] => [-1 - val//dimension, -1 - val%dimension]

if nr_zone is 64 or 100 => your code work good, because 'difference' variable's value is 0.

However, it was confirmed that it does not work well when the value of nr_zone is not a square number and the value of the 'difference' variable is derived larger than the 'dimension' variable determined at the same time.

example :
If nr_zone is 105. In an array of size 11 by 11, 16 spaces must be set to none. In that case, your code may generate an error where the index is exceeded.

@kimikitae kimikitae reopened this Aug 28, 2024
Copy link
Member
@Krien Krien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kimikitae I checked the changes locally and they look good to me. Good solution.
As a sidenote the text in reset_ctr plot does look invisible for non-square dimensions, I might need to fix that later.

@Krien
Copy link
Member
Krien commented Sep 11, 2024

Also thank you for your contribution. I hope the tool will be valuable in your work.

@Krien Krien merged commit 0701942 into stonet-research:master Sep 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants