-
Notifications
You must be signed in to change notification settings - Fork 13
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
The correct way to select cells? #4
Comments
Hi, I tried to fix this issue. Could you please test whether it is okay now? |
Sorry, I don't know why you delete this code? I think the NA data still can not be filtered. Line 86 in d8d492d
|
selected_idx is a vector indicating which index contains NA value, but it has the index of all the data (length=1280). so it is impossible to filter out NA values by using its index. it would help if you changed it into |
I see, tried to fix it in the previous commit |
@juychen @Wang-Cankun @PegasusAM @OSU-BMBL-admin Could you please give some explanation about these bugs in your project code? |
Some issues may happen when merging from the development branch. We are checking the issue at the moment. We will fix it soon |
Hi @juychen, Do you release the final version of your code? I have a question why did you commit this line? Line 86 in 8adedff
Has the bug in the code been resolved? Thanks for the adata results you provided. Can you provide a bash file to get your adata file from the current code? 请问代码的Bug解决了吗?感谢你们提供你们跑出来的结果。但能否提供一个从现在代码得到你们adata文件的运行文件?这是复现结果很重要的一步。 |
|
Yes. I clone the repository. I still have the bug. @juychen
Line 109 in 8adedff
|
Hi, I have fixed the corresponding bug by adding the fillna code. Also, I have edited the code to enhance usability, including the selection of CPU devices and making result folders if not exists. Updates are now pushed to the main branch and it should work correctly now. I have tested starting the configuration from scratch by the following procedure: create a new folder, clone code from GitHub, download the data, and run code following the command line. You can clean up the previous version and start from scratch to see whether it works. |
Thanks! @juychen. So could you reproduce the same h5ad results(https://portland-my.sharepoint.com/:u:/g/personal/junyichen8-c_my_cityu_edu_hk/EYru-LaQC1tHlFZSnf1RA_cBjXwIafy-iDsajEWjh8xcjA?e=2sE61e) by using the new code? |
I reproduced three datasets based on the parameters of the author's adata file, and found that the final results were very poor, and I felt that it was necessary for the author to double-check the code and data |
Yes, I've noticed that the results were indeed poor and far from what's described in the article. This certainly necessitates a thorough review of the code and data by the authors to ensure accuracy. |
I am confused is this the right way to get the selected index?
In your code, you define the
selected_idx
as:scDEAL/bulkmodel.py
Line 109 in a2cf462
and you use it as:
scDEAL/bulkmodel.py
Line 118 in a2cf462
but actually
selected_idx
(the output of line 109) still contains all the index, like this:so it means that in line 118 the
selected_idx.index
still have all the ids and this code failed to filters the data with na. And I suspect this also cause the error mentioned in other issues like #3. Could you please recheck your code and results? Thanks.The text was updated successfully, but these errors were encountered: