amkrajewski / nimCSO

nim Composition Space Optimization is a high-performance tool leveraging metaprogramming to implement several methods for selecting components (data dimensions) in compositional datasets, as to optimize the data availability and density for applications such as machine learning.
https://nimcso.phaseslab.org
MIT License
23 stars 1 forks source link

JOSS Review Comments #5

Closed atzberg closed 3 weeks ago

atzberg commented 2 months ago

Overall the package is well organized with clear documentation, installation instructions, and examples. Below are a few relatively small items that were encountered:

atzberg commented 2 months ago
atzberg commented 2 months ago
RMeli commented 1 month ago

https://github.com/openjournals/joss-reviews/issues/6731

amkrajewski commented 1 month ago
  • The apt-get install nim did not work on platform $ uname -a Linux XXX 6.5.0-41-generic #41~22.04.2-Ubuntu SMP PREEMPT_DYNAMIC Mon Jun 3 11:32:55 UTC 2 x86_64 x86_64 x86_64 GNU/Linux Perhaps can clarify for which distributions package nim is available with this installation method. The Conda based installation worked fine here, and the documentation instructions do also give several other ways to install the package.

I adjusted the install instructions in 85ab1b0, doing my best to present Conda as the preferred installation method on Linux and highlighting that the default channel/repository may have an outdated version not supported with nimCSO. I also added tests for old nim=1.6 to make sure it works, in case someone tries to use the old version anyway.

amkrajewski commented 1 month ago
  • Running the quickstart.ipynb jupyter notebook encounters issue with finding the YAML configuration file, even though it is in the same local directory. !nim c -f -d:release -d:configPath=config.yaml --out:nimcso ../src/nimcso Hint: used config file 'XXX/anaconda3/nim/config/nim.cfg' [Conf] Hint: used config file 'XXX/anaconda3/nim/config/config.nims' [Conf] .................................................................................................................................. XXX/github_nimCSO__JOSS_review/src/nimcso.nim(37, 8) Error: cannot open file: yaml

I haven't encountered this problem before, but I believe you are experiencing a missing yaml package, rather than a missing YAML configuration file. If configuration could not be found, I would expect an IO exception like:

Error: unhandled exception: cannot open: configThatDoesNotExist.yaml [IOError]

Could you please double-check that yaml is present in the output of nimble list -i?

atzberg commented 1 month ago

yaml is now installed, but issues persist.
Confirmed install by conda list | grep yaml.

Still issues persist in the jupyter notebook:

!nimble list -i (shows blank) [also when run from command line]

!nim c -f -d:release -d:configPath=config.yaml --out:nimcso ../src/nimcso usage: nim [-h] [--verbose] [--version] [--config FILE] {show} ... nim: error: argument subparser_level_0: invalid choice: 'c' (choose from 'show')

!nim --version version: 0.2.0

Hope that helps in sorting the issues out with the example jupyter notebook.

amkrajewski commented 1 month ago

@atzberg Thanks for the above information; it greatly narrows possible issues!

I hope the above makes sense, but please do not hesitate to ask for clarification!

atzberg commented 1 month ago

OK, that explains the issue. Would be good to add comment on this to the install instructions in the documents. The package looks good here to accept and publish.

RMeli commented 1 month ago

@atzberg, does 4127cbb address your comments and solves this issue?

atzberg commented 1 month ago

Yes

On Mon, Aug 12, 2024, 1:46 PM Rocco Meli @.***> wrote:

@atzberg https://github.com/atzberg, does 4127cbb https://github.com/amkrajewski/nimCSO/commit/4127cbbabb51d26ace0cb10700978ef6a8b6f940 address your comments and solves this issue?

— Reply to this email directly, view it on GitHub https://github.com/amkrajewski/nimCSO/issues/5#issuecomment-2284876449, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBHSZWA6VHZQD4CY4J2Q7LZRENI3AVCNFSM6AAAAABKTLIUP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBUHA3TMNBUHE . You are receiving this because you were mentioned.Message ID: @.***>

RMeli commented 1 month ago

Thank you for the update. @amkrajewski looks like this issue can be closed.