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

Exposing when bellhop.exe is not recognized #103

Open
ctuguinay opened this issue Apr 26, 2024 · 2 comments
Open

Exposing when bellhop.exe is not recognized #103

ctuguinay opened this issue Apr 26, 2024 · 2 comments
Assignees

Comments

@ctuguinay
Copy link

ctuguinay commented Apr 26, 2024

Hey ARL team,

I was trying to use .compute_eigenrays and nothing actually happened (which was indicated by .plot_rays error-ing out), and then I tried again with debug turned on and all I got was:

[DEBUG] Model: bellhop
[WARN] Bellhop did not generate expected output file

This was not too helpful in the debugging process.

I was looking back at the code to see where bellhop.exe is called and I printed out the result of _proc.run of bellhop and got the following:

CODE:

    def _bellhop(self, *args):
        try:
            result = _proc.run(f'bellhop.exe {" ".join(list(args))}',
                            stderr=_proc.STDOUT, stdout=_proc.PIPE,
                            shell=True)
            print(result)

OUTPUT:

CompletedProcess([Some other args here with personal paths].......returncode=1, stdout=b"'bellhop.exe' is not recognized as an internal or external command,\r\noperable program or batch file.\r\n")

With this message, I was able to fix the problem (which ended up being a path issue to bellhop.exe), but I wonder now if stdout when returncode=1 should be exposed to the user during debug is True or just raised as some sort of warning? If any of these sound good, I'd be willing to submit a PR with this proposed change.

Edit: I tested this via the bellhop.ipynb notebook, so perhaps these messages are exposed in python scripts?

@mchitre
Copy link
Member

mchitre commented Apr 29, 2024

Yes, a warning when something fails would be good. Happy to accept a PR.

@Arthanor
Copy link

Hi, I don't know if it's because I have a wrong version (I have 1.8.4, from anaconda), but in my version, this has not been implemented, so the path error doesn't get reported and it just seems like bellhop failed.

I think that _models.append(('bellhop', _Bellhop)) on line 854 of uwapm.py, also makes it impossible for the typical pm.models() test to no list bellhop as well.

Combined, it makes it difficult for new users to identify path errors since, it seems like bellhop is there from the models list and, when trying to run it, the error is that it failed to generate files, not that it wasn't found. For such a common/basic error, it shouldn't be necessary to dig in and view internal function messages to know what is happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants