-
Notifications
You must be signed in to change notification settings - Fork 4
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
rotary bed notebooks #3
base: main
Are you sure you want to change the base?
Conversation
Please open a PR into Pyomo with sufficient background suggestions for the Pyomo team to add feedback. Make sure the proposed changes are general and not one-off modifications just for this case study. |
@bpaul4 @Ryan-Hughes-8 Please see this PR for the measurement optimization and MBDoE code based on the RPB mdel. |
Hi @jialuw96. Both @Ryan-Hughes-8 and I tested the workflow, and were unable to move past Step 3 due to Git being unable to locate your branch:
|
(Sorry for "crashing the PR"---I happened to see this comment in my GH notifications) Does this # install from head branch
pip install "git+https://github.com/jialuw96/pyomo@RPB_MBDOE"
# alternatively, install directly from PR URL
pip install "git+https://github.com/Pyomo/pyomo@refs/pull/3250/merge" |
@lbianchi-lbl the modified installation call worked, thank you! |
The installation instructions are updated. Other updates for this PR include: Pyomo modification
Readme file
Counter-current flow model (Results are represented in Jialu's thesis chapter 4)
|
Hi @jialuw96, I am attempting to follow the workflow in the readme file, but there is an issue in Step 1 of the Run measurement optimization results section. In
|
@Ryan-Hughes-8 can you try again and see if the latest set of commits after your original attempt resolve the errors you have experienced? |
Hi @jialuw96, I got a chance to attempt to go through the steps laid out in the README for running measurement optimization results. I received a few errors and have a couple comments.
|
Is @jialuw96 still available to work on this? I've heard she's graduated. |
Hi @Ryan-Hughes-8 Thank you for the feedbacks! I have made some changes and committed them. Please see the following for answers to your suggestions specifically:
Thank you for testing the code! I am keeping an eye on the repo and will make my best to answer in time. |
Hi @jialuw96, thank you for your recent modifications. I attempted to run through the README, and while the installation went smoothly I encountered some issues with the notebooks and made some changes locally to resolve the issues where possible. In general, the notebooks should have some comments explaining what the Input cells are doing. Counterflow_MO_data_process.ipynb Cocurrent_flow_MBDoE.py Cocurrent_flow_MBDoE draw_figure.ipynb called draw_figures in the README, notebook runs smoothly with no issues |
Hi @bpaul4 , thank you for the changes; Please see my feedbacks in the following. Overall I found there are some file names you mentioned that I cannot find; To work on the same contents, I am working on the code in this PR on 70b173b, is there any commit in other branches or other PR?
Yes, this is the correct object and is the way to achieve total_fim.
That works; That is a debugging function and I just kept it in case.
I am guessing you are mentioning
This seems like a gurobi environment issue; There is a more detailed Gurobi installation guidance for this problem (Readme, step 5) here. I am guessing installing
Yes, those were for debugging and are not used any more.
It should be
It should be './Cocurrent_MO_QVs/Var_z3'.
I cannot find this file; Could you check the file name please?
I also cannot find RPB_model_kaug_merge; I am able to download kaug_testcase.ipynb and see the contents in the notebook. Thank you for your updates! |
@jialuw96 thank you for your clarifications. I ran through the README again this afternoon. I was unable to test CountercurrentMBDoE.ipynb or kaug_test.ipynb, encountered an error about having not have enough memory to run the file from - this happened with both the browser and console commands. Thank you for the note on Cocurrentflow_MBDoE.ipynb. The section "Measurements and their variances" should be updated to:
In the same notebook, the section "Solve a square MBDoE problem with k_aug" fails due to what seems to be Pyomo compatibility issue:
I have Pyomo version 6.7.3 (the latest version is 6.8.0). Would you be able to make the changes discussed here and my prior comment, and test the files on your local machine? |
Hello @jialuw96, have there been any updates on this? |
@adowling2 this is a pretty big PR (nearly 600 files changed) and has been open for several months now. Might it be possible merge this (or some subset of it) in and not leave such large set of pending changes open on PR/branch? |
I like this plan. Merge it and then we can have a PR to clean it up.
… Message ID: ***@***.***
com>
|
No description provided.