Flux consumption

[IsaacSavona]

Added inductance functionality and flux functionality to POPCON for time-dependent and time-independent calculations.

[hassec]

Hi @IsaacSavona,

👍 on the PR, I've left a first round of comments.
Will continue later after I'm done with meetings.

CI checks seem to have failed due to some test failures which need be fixed.

[IsaacSavona]

Hi @IsaacSavona,

👍 on the PR, I've left a first round of comments. Will continue later after I'm done with meetings.

CI checks seem to have failed due to some test failures which need be fixed.

I made a commit where I made a separate yaml input file just for my tutorial on inductances and fluxes. Let me know if this solves those CI check issues. Additionally, my most recent commit resolves the glossary and documentation issues you have raised here.

[hassec]

@IsaacSavona, another CI run just finished and still shows some errors, could you have a look?

[IsaacSavona]

@IsaacSavona, another CI run just finished and still shows some errors, could you have a look?

It seems like test_regression_against_cases.py is failing because my named_option (i.e., Barr) of SurfaceInductanceCoeffs within the SPARC_PRD_fluxes_inductances, which is input as surface_inductance_coefficients: Barr in the .yaml, passes the string 'Barr' which fails the equality test at the very beginning of the file \formulas\inductances.py since a str is not a name. I am not sure how to fix this because it seems like the .yaml input for this named option is the same in the .yaml file as, for example, prf. Any suggestions?

[tbody-cfs]

@IsaacSavona, another CI run just finished and still shows some errors, could you have a look?

It seems like test_regression_against_cases.py is failing because my named_option (i.e., Barr) of SurfaceInductanceCoeffs within the SPARC_PRD_fluxes_inductances, which is input as surface_inductance_coefficients: Barr in the .yaml, passes the string 'Barr' which fails the equality test at the very beginning of the file \formulas\inductances.py since a str is not a name. I am not sure how to fix this because it seems like the .yaml input for this named option is the same in the .yaml file as, for example, prf. Any suggestions?

Hi @IsaacSavona,
Could you add handling for SurfaceInductanceCoeffs to https://github.com/IsaacSavona/cfspopcon/blob/flux_consumption/cfspopcon/helpers.py#L18

[IsaacSavona]

@IsaacSavona, another CI run just finished and still shows some errors, could you have a look?

It seems like test_regression_against_cases.py is failing because my named_option (i.e., Barr) of SurfaceInductanceCoeffs within the SPARC_PRD_fluxes_inductances, which is input as surface_inductance_coefficients: Barr in the .yaml, passes the string 'Barr' which fails the equality test at the very beginning of the file \formulas\inductances.py since a str is not a name. I am not sure how to fix this because it seems like the .yaml input for this named option is the same in the .yaml file as, for example, prf. Any suggestions?

Hi @IsaacSavona, Could you add handling for SurfaceInductanceCoeffs to https://github.com/IsaacSavona/cfspopcon/blob/flux_consumption/cfspopcon/helpers.py#L18

Added this. Solved some issues. Take a look at my new commit. I am trying to get regression test cases to work. How do I add values to the right hand side that exist on the left hand side (e.g., PF_flux, external inductance, etc.) in the SPARC_PRD_fluxes_inductances_result.nc file?

[hassec]

@IsaacSavona
It seems that you will need to run python tests/regression_results/generate_regression_results.py to update the reference files. Please also note that you'll have to run git add --force tests/regression_results/SPARC_PRD_fluxes_inductances_result.nc to add the new file as *.nc are by default ignored.

If I do that, the PRD case works and the PRD+fluxes case is able to find its reference, but fails due to errors in not matching units, which likely is due to errors in cfspopcon/unit_handling/default_units.py

[hassec]

@IsaacSavona I've just pushed a commit that shows the required fixes 😉

[IsaacSavona]

@IsaacSavona It seems that you will need to run python tests/regression_results/generate_regression_results.py to update the reference files. Please also note that you'll have to run git add --force tests/regression_results/SPARC_PRD_fluxes_inductances_result.nc to add the new file as *.nc are by default ignored.

If I do that, the PRD case works and the PRD+fluxes case is able to find its reference, but fails due to errors in not matching units, which likely is due to errors in cfspopcon/unit_handling/default_units.py

I have made all of these changes and I am trying to push but am getting the following:

> git pull --tags origin flux_consumption
From https://github.com/IsaacSavona/cfspopcon
 * branch            flux_consumption -> FETCH_HEAD
hint: You have divergent branches and need to specify how to reconcile them.
hint: You can do so by running one of the following commands sometime before
hint: your next pull:
hint: 
hint:   git config pull.rebase false  # merge
hint:   git config pull.rebase true   # rebase
hint:   git config pull.ff only       # fast-forward only
hint: 
hint: You can replace "git config" with "git config --global" to set a default
hint: preference for all repositories. You can also pass --rebase, --no-rebase,
hint: or --ff-only on the command line to override the configured default per
hint: invocation.
fatal: Need to specify how to reconcile divergent branches.

[IsaacSavona]

Yes. Just accidentally clicked on it.

…

On Nov 17, 2023, at 3:04 PM, Christoph Hasse ***@***.***> wrote: @hassec commented on this pull request. On example_cases/time_independent_flux_comp.py <#4 (comment)>: @IsaacSavona <https://github.com/IsaacSavona> this file seems to have been added accidentally? — Reply to this email directly, view it on GitHub <#4 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APWATPKR5EJOT45J25ZCWKDYE5VH5AVCNFSM6AAAAAA7B3I7GCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMZXGA3TENJZG4>. You are receiving this because you were mentioned.

[tbody-cfs]

Replaced by #57