search

QSD-Group / EXPOsan

EXPOsition of sanitation and resource recovery systems
https://qsdsan.com
Other
16 stars 6 forks source link

correct MFSP calculation #33

Closed jiananf2 closed 1 year ago

jiananf2 commented 1 year ago

corrected MFSP to $/GGE

yalinli2 commented 1 year ago

thanks @jiananf2 ! the tests failed but it's just we need to update the numbers in the tests

actually, it'll be good if you can run the tests locally, can you try setting up the local testing following these instructions: https://qsdsan.readthedocs.io/en/latest/CONTRIBUTING.html#testing

if you manage to get it work, I can give you the write access to the main branch so you can directly push (after making sure the local tests can pass) and don't need to open PR, thanks!

jiananf2 commented 1 year ago

Will work on that and I am going to close this PR, thanks!

jiananf2 commented 1 year ago

Hi @yalinli2, the local testing has passed!

yalinli2 commented 1 year ago

thanks @jiananf2 for trying the local tests out! but seems like there are conflicts between main and pfas, also the online tests have failed, can you look into it before merging?

the online tests can fail for a bunch of weird reasons and quite often it's about configuration rather than bugs... let me know if you can't get it work, thanks!

yalinli2 commented 1 year ago

@jiananf2 did you test with the changes reverted back as this? https://github.com/BioSTEAMDevelopmentGroup/thermosteam/commit/7b4468929a654f0ea78b3fcfdc80da659ee2b926#comments

i.e., take out the try... except... block and just use 

    x = flx.wegstein(x_iter, x_guess, 1e-10, args=args, checkiter=False,
                     checkconvergence=False, convergenceiter=3)

the same error showed up when I did this locally...

thanks!

jiananf2 commented 1 year ago

I did with the changes in thermosteam. I made change in QSDsan and EXPOsan. Did you use pfas branches in both?

From: Yalin @.> Sent: Friday, February 3, 2023 5:55:47 PM To: QSD-Group/EXPOsan @.> Cc: Feng, Jianan @.>; Mention @.> Subject: Re: [QSD-Group/EXPOsan] correct MFSP calculation (PR #33)

@jiananf2https://urldefense.com/v3/__https://github.com/jiananf2__;!!DZ3fjg!4VXQIvvU0S93S6rn6Qzg8hpsLDgmm8b2kOqT57iRc0TSJGE87hCyIuX5R2_jGpz5FhyGDAbgBMzK0EL7yHFXYQPZNonC$ did you test with the changes reverted back as this? @.**#comments<https://urldefense.com/v3/__https://github.com/BioSTEAMDevelopmentGroup/thermosteam/commit/7b4468929a654f0ea78b3fcfdc80da659ee2b926comments__;Iw!!DZ3fjg!4VXQIvvU0S93S6rn6Qzg8hpsLDgmm8b2kOqT57iRc0TSJGE87hCyIuX5R2_jGpz5FhyGDAbgBMzK0EL7yHFXYYKTVjXj$>

i.e., take out the try... except... block and just use

x = flx.wegstein(x_iter, x_guess, 1e-10, args=args, checkiter=False,
                 checkconvergence=False, convergenceiter=3)

the same error showed up when I did this locally...

thanks!

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/QSD-Group/EXPOsan/pull/33*issuecomment-1416540214__;Iw!!DZ3fjg!4VXQIvvU0S93S6rn6Qzg8hpsLDgmm8b2kOqT57iRc0TSJGE87hCyIuX5R2_jGpz5FhyGDAbgBMzK0EL7yHFXYWMkOnLy$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/A27WI3RS7KRKFCNJ464CABDWVWLIHANCNFSM6AAAAAAT5CTVMI__;!!DZ3fjg!4VXQIvvU0S93S6rn6Qzg8hpsLDgmm8b2kOqT57iRc0TSJGE87hCyIuX5R2_jGpz5FhyGDAbgBMzK0EL7yHFXYWFNDGv9$. You are receiving this because you were mentioned.Message ID: @.***>

yalinli2 commented 1 year ago

ohhhh I didn't know you made updates in QSDsan, let me try again, thanks!