Skip to content

Fixed #3150 (winding pack current density constraint)#3160

Merged
timothy-nunn merged 3 commits intomainfrom
3150-incorrect-error-in-winding-pack-current-density-constraint
Jun 12, 2024
Merged

Fixed #3150 (winding pack current density constraint)#3160
timothy-nunn merged 3 commits intomainfrom
3150-incorrect-error-in-winding-pack-current-density-constraint

Conversation

@mkovari
Copy link
Copy Markdown
Collaborator

@mkovari mkovari commented Apr 25, 2024

Fixed #3150.
Removed obsolete references to PROCESS Users's Guide.

@mkovari mkovari linked an issue Apr 25, 2024 that may be closed by this pull request
Incorrect error in winding pack current density constraint. #3150
Closed
jonmaddock
jonmaddock requested changes Apr 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@jonmaddock jonmaddock left a comment

Choose a reason for hiding this comment

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

I think there's some indentation that needs correcting. Also, why has an image been put in the examples directory? Perhaps this belongs in the docs?

Comment thread process/current_drive.py Outdated
neutral beam system, based on the 1990 ITER model.
AEA FUS 251: A User's Guide to the PROCESS Systems Code
ITER Physics Design Guidelines: 1989 [IPDG89], N. A. Uckan et al,
ITER Physics Design Guidelines: 1989 [IPDG89], N. A. Uckan et al,
Copy link
Copy Markdown
Contributor

@jonmaddock jonmaddock Apr 26, 2024

Choose a reason for hiding this comment

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

Fix indentation please.

Comment thread process/evaluators.py Outdated
constraints in conf.
AEA FUS 251: A User's Guide to the PROCESS Systems Code
:param n: number of variables
:param n: number of variables
Copy link
Copy Markdown
Contributor

@jonmaddock jonmaddock Apr 26, 2024

Choose a reason for hiding this comment

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

Again, fix indentation please.

@mkovari
Copy link
Copy Markdown
Collaborator Author

mkovari commented Apr 26, 2024 •
edited
Loading

The image files plot_proc_1, 2 and 3 must have been put into the examples folder automatically when I ran pytest. Because there were so many modified files I used git commit -a, and because I didn't have the examples folder open in VSCode I didn't see they had been changed.

However, they seem to fit, and since only the new third page is shown in the changes tab, I suppose that the other two pages, plot_proc_1.png and plot_proc_2.png were already there, so I have left plot_proc_3.png there.

@mkovari
Copy link
Copy Markdown
Collaborator Author

mkovari commented Apr 26, 2024 •
edited
Loading

I have corrected indentations in several files.

@mkovari mkovari changed the title Fixed #3150 (winding pack current densoty constraint) Fixed #3150 (winding pack current density constraint) Apr 26, 2024
@mkovari mkovari marked this pull request as draft April 26, 2024 14:38
@mkovari mkovari marked this pull request as ready for review April 26, 2024 15:06
@mkovari mkovari requested a review from jonmaddock April 26, 2024 15:06
jonmaddock
jonmaddock previously requested changes May 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@jonmaddock jonmaddock left a comment

Choose a reason for hiding this comment

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

Please fixup the changes to comments to eliminate the 2 correcting commits and eliminate churn. The plot_procX.png figures should be created by running the examples.ipynb notebook, and the resulting pngs displayed in the notebook. Please look at how figure 1 and 2 are being handled by the notebook to match. Thanks!

@mkovari
Copy link
Copy Markdown
Collaborator Author

mkovari commented May 7, 2024

The plot_proc figures have nothing to do with this pull request. They result from Chris's edit. If the example notebook needs changing it should be done there. I will delete the unwanted png.

@mkovari mkovari force-pushed the 3150-incorrect-error-in-winding-pack-current-density-constraint branch from bb88e51 to 10814bc Compare May 9, 2024 13:52
@mkovari
Copy link
Copy Markdown
Collaborator Author

mkovari commented May 9, 2024

Unwanted png removed. Rebased.

@mkovari
Copy link
Copy Markdown
Collaborator Author

mkovari commented May 14, 2024

I have fixed the conflict localy, but when I rebased and then tried to force push, I couldn't do it:

(env) mkovari@J1714:~/PROCESS$ git push --force
ssh: connect to host github.com port 22: Connection timed out
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@mkovari mkovari force-pushed the 3150-incorrect-error-in-winding-pack-current-density-constraint branch from 10814bc to b1db908 Compare May 14, 2024 12:45
timothy-nunn
timothy-nunn previously requested changes Jun 12, 2024
View reviewed changes
Comment thread source/fortran/init_module.f90 Outdated


!! AEA FUS 251: A User's Guide to the PROCESS Systems Codefile:///home/mkumar/process/source/fortran/divertor_ode.f90
!! file:///home/mkumar/process/source/fortran/divertor_ode.f90
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn Jun 12, 2024

Choose a reason for hiding this comment

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

Can we just remove this line, that file is long gone

@mkovari mkovari dismissed stale reviews from timothy-nunn and jonmaddock June 12, 2024 10:45

COmpleted

@mkovari mkovari requested a review from timothy-nunn June 12, 2024 10:46
@timothy-nunn timothy-nunn merged commit abc2980 into main Jun 12, 2024
@timothy-nunn timothy-nunn deleted the 3150-incorrect-error-in-winding-pack-current-density-constraint branch June 12, 2024 14:36
chris-ashe pushed a commit that referenced this pull request Jul 8, 2024
@mkovari @chris-ashe
Fixed #3150 (winding pack current density constraint) (#3160)
309db54 
* Fixed #3150 (winding pack current densoty constraint). Removed obsolete reference to Users' Guide.

* Removed two obsolete comments

---------

Co-authored-by: Michael Kovari <michael.kovari@ukaea.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timothy-nunn timothy-nunn timothy-nunn approved these changes

@jonmaddock jonmaddock jonmaddock left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect error in winding pack current density constraint.

3 participants

@mkovari @timothy-nunn @jonmaddock