NREL / GEOPHIRES-X

MIT License
28 stars 24 forks source link

MC doesn't work for HIP-RA #151

Closed softwareengineerprogrammer closed 6 months ago

softwareengineerprogrammer commented 6 months ago

As documented in disabled unit test: https://github.com/NREL/GEOPHIRES-X/blob/d821772a2e13397983343ad7ca710bc2d90a118a/tests/geophires_monte_carlo_tests/test_geophires_monte_carlo.py#L110-L126

And marked by FIXME/TODO: https://github.com/NREL/GEOPHIRES-X/blob/43110be367f7ff3e3ce94d66a61fb32ca19ef1db/src/geophires_monte_carlo/MC_GeoPHIRES3.py#L339-L345


Steps to address:

  1. Enable test_hip_ra_monte_carlo unit test (remove line 110 of test_geophires_monte_carlo.py)
  2. Fix HIP-RA result parsing implementation - FIXME in line 339 of MC_GeoPHIRES3.py indicates probable best place to start
  3. Verify fix works by test_hip_ra_monte_carlo unit test passing
  4. Optional but encouraged - add additional assertions to test to increase coverage
  5. Submit PR with fix
malcolm-dsider commented 6 months ago

I saw this "FIXME TODO" about HIP-RA, so I switched to trying to run GEOPHIRES with MC_GEOPHIRES. It fails for the same reason.

softwareengineerprogrammer commented 6 months ago

@malcolm-dsider What error are you getting locally? Do the unit tests pass for you locally?

MC unit tests are passing on all python versions/platforms in Actions - see for example https://github.com/NREL/GEOPHIRES-X/actions/runs/8224105359/job/22487543700#step:5:63. (MC also works locally on my Mac both in unit tests and other programs I've written that consume it as a pip package)

malcolm-dsider commented 6 months ago

The MC tests are passing, but they don’t run from the command line. Something is wrong with the way arguments are being passed or something?!?!? IDK. The unittest framework runs the tests differently than the way things run from the command line. But I can do my coding by running my development test from within the unittest framework, so I can make progress without bothering you tomorrow.

I will cancel the meeting, set a bug in the GitHub bug reporting framework, and move on. We can look at it together when you have more time.

From: Jonathan Pezzino @.> Sent: Sunday, March 10, 2024 3:18 PM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] MC doesn't work for HIP-RA (Issue #151)

@malcolm-dsider https://github.com/malcolm-dsider What error are you getting locally? Do the unit tests pass for you locally?

MC unit tests are passing on all python versions/platforms in Actions - see for example https://github.com/NREL/GEOPHIRES-X/actions/runs/8224105359/job/22487543700#step:5:63. (MC also works locally on my Mac both in unit tests and other programs I've written that consume it as a pip package).

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/issues/151#issuecomment-1987350500 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYTZEQ73M4CSZU4VU4GTYXTEZXAVCNFSM6AAAAABEPHEHV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGM2TANJQGA . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYT63S7ZBDSXTHNMQ6E3YXTEZXA5CNFSM6AAAAABEPHEHV6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWOSH6I.gif Message ID: @. @.> >

softwareengineerprogrammer commented 6 months ago

@malcolm-dsider Does the approach in the (very basic) MC user guide work for you? https://softwareengineerprogrammer.github.io/GEOPHIRES-X/Monte-Carlo-User-Guide.html

malcolm-dsider commented 6 months ago

That is the approach I am using temporarily to make progress, but we need to fix the command line eventually.

From: Jonathan Pezzino @.> Sent: Sunday, March 10, 2024 3:57 PM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] MC doesn't work for HIP-RA (Issue #151)

@malcolm-dsider https://github.com/malcolm-dsider Does the approach in the (very basic) MC user guide work for you? https://softwareengineerprogrammer.github.io/GEOPHIRES-X/Monte-Carlo-User-Guide.html

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/issues/151#issuecomment-1987359564 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AWGVYT2QD6TOHLSXX2UOTMDYXTJLDAVCNFSM6AAAAABEPHEHV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGM2TSNJWGQ . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AWGVYT7UCZOJ2HO4VRD3GYLYXTJLDA5CNFSM6AAAAABEPHEHV6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWOSZUY.gif Message ID: @. @.> >

softwareengineerprogrammer commented 6 months ago

What command are you using on the command line and what is the resulting error message?

On Sun, Mar 10, 2024 at 13:59 Malcolm Ross @.***> wrote:

That is the approach I am using temporarily to make progress, but we need to fix the command line eventually.

From: Jonathan Pezzino @.> Sent: Sunday, March 10, 2024 3:57 PM To: NREL/GEOPHIRES-X @.> Cc: Malcolm Ross @.>; Mention @.> Subject: Re: [NREL/GEOPHIRES-X] MC doesn't work for HIP-RA (Issue #151)

@malcolm-dsider https://github.com/malcolm-dsider Does the approach in the (very basic) MC user guide work for you? https://softwareengineerprogrammer.github.io/GEOPHIRES-X/Monte-Carlo-User-Guide.html

— Reply to this email directly, view it on GitHub < https://github.com/NREL/GEOPHIRES-X/issues/151#issuecomment-1987359564> , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AWGVYT2QD6TOHLSXX2UOTMDYXTJLDAVCNFSM6AAAAABEPHEHV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGM2TSNJWGQ> . You are receiving this because you were mentioned. < https://github.com/notifications/beacon/AWGVYT7UCZOJ2HO4VRD3GYLYXTJLDA5CNFSM6AAAAABEPHEHV6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTWOSZUY.gif> Message ID: @. @.> >

— Reply to this email directly, view it on GitHub https://github.com/NREL/GEOPHIRES-X/issues/151#issuecomment-1987360193, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA66IPEYO2ZDXTS3TPJPITDYXTCT3AVCNFSM6AAAAABEPHEHV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGM3DAMJZGM . You are receiving this because you authored the thread.Message ID: @.***>

softwareengineerprogrammer commented 6 months ago

<Discussion re: command line issues moved to https://github.com/NREL/GEOPHIRES-X/issues/153>