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

Build package updates #643

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

hellkite500
Copy link
Contributor

Some build/package updates

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

Looks good for the most part. I left a few minor questions and comments that should be easy to work through quickly. Thanks!

@@ -112,7 +112,7 @@ def build_extensions(self):
sources=[
"troute/network/reservoirs/rfc/rfc.{}".format(ext),
],
include_dirs=[np.get_include()],
include_dirs=[np.get_include(), "troute/network/"],
Copy link
Member

Choose a reason for hiding this comment

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

Is this an issue if you try to install from a different directory than the dir that has setup.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about that, haha.

Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be in this PR? Seems unrelated. Seems related to #616.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was based on that branch, that is a shared commit 592afa7 so I can rebase this PR to clear it out if needed.

@shorvath-noaa
Copy link
Contributor

@hellkite500 is this PR still being worked on or is it ready for review/merge?

@hellkite500
Copy link
Contributor Author

@shorvath-noaa It should be good as is. Might need to do some testing based on @aaraney s comment and make some adjustments, but that can be done separately.

@shorvath-noaa
Copy link
Contributor

@hellkite500 I got this to compile ok, but when I run t-route with RFC's on it still fails, so I'm not sure this fixes Issue 620. This is the error I get:

At line 23 of file bind_rfc.f90
Fortran runtime error: Attempt to DEALLOCATE unallocated 'rfc_ptr'
At line 23 of file bind_rfc.f90
Fortran runtime error: Attempt to DEALLOCATE unallocated 'rfc_ptr'
Traceback (most recent call last):
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/sean.horvath/projects/t-route/src/troute-nwm/src/nwm_routing/__main__.py", line 2028, in <module>
    main_v04(v_args[1])
  File "/home/sean.horvath/projects/t-route/src/troute-nwm/src/nwm_routing/__main__.py", line 161, in main_v04
    run_results = nwm_route(
  File "/home/sean.horvath/projects/t-route/src/troute-nwm/src/nwm_routing/__main__.py", line 1074, in nwm_route
    results = compute_nhd_routing_v02(
  File "/home/sean.horvath/projects/t-route/src/troute-routing/troute/routing/compute.py", line 621, in compute_nhd_routing_v02
    results_subn[order] = parallel(jobs)
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/joblib/parallel.py", line 1061, in __call__
    self.retrieve()
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/joblib/parallel.py", line 938, in retrieve
    self._output.extend(job.get(timeout=self.timeout))
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/joblib/_parallel_backends.py", line 542, in wrap_future_result
    return future.result(timeout=timeout)
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/concurrent/futures/_base.py", line 446, in result
    return self.__get_result()
  File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/concurrent/futures/_base.py", line 391, in __get_result
    raise self._exception
joblib.externals.loky.process_executor.TerminatedWorkerError: A worker process managed by the executor was unexpectedly terminated. This could be caused by a segmentation fault while calling the function or by an excessive memory usage causing the Operating System to kill the worker.

The exit codes of the workers are {EXIT(2)}

@hellkite500 hellkite500 mentioned this pull request Oct 7, 2023
@hellkite500
Copy link
Contributor Author

@shorvath-noaa This fixes the build/packaging -- what you are seeing is a runtime error that is a separate issue, I think.

Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

Verified this installed correctly with python3.8 on Debian slim bullseye.

@shorvath-noaa shorvath-noaa merged commit ed16f4e into NOAA-OWP:master Oct 10, 2023
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.

3 participants