Armadillo by aaronjridley · Pull Request #31 · AetherModel/Aether

aaronjridley · 2021-01-06T19:05:38Z

Description

Addresses #29 (at least partially, issue linked by @aburrell )

This brings in the changes of the code to go from 1d c-native arrays covering all three dimensions (with loops and look ups) to 3d armadillo arrays, which can be addressed directly (i, j, k) or through loop-less array math.

Type of change

New feature (non-breaking change that adds functionality)
This change requires a documentation update

How Has This Been Tested?

Yes, when you compare a 30 minute simulation with the c-native arrays and the armadillo arrays, they are very similar to each other, providing both thermospheric heating and ionization in the right places with the right magnitudes.

Test configuration

OSX
gcc version 9.3.0
Need armadillo and netcdf libraries installed

Checklist:

Make sure you are merging into the develop (not master) branch
My code follows the style guidelines of this project
I have performed a self-review of my own code
[Sort of] I have commented my code, particularly in hard-to-understand areas
[Need to do more] I have made corresponding changes to the documentation
My changes generate no new warnings
[Sort of] I have added tests that prove my fix is effective or that my feature works
[Need to make more unit tests] New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
[N/A] Add a note to CHANGELOG.md, summarizing the changes

…lts.

aburrell

Some comments, mostly on areas I'd like more comments :P. It'd probably be a good idea to run the files through a linter, too. I've never linted C/C++ code so any of my style suggestions are suspect. I made a few anyway, though, based on the dominant style of the current code.

include/grid.h

include/ions.h

include/neutrals.h

include/sizes.h

src/calc_neutral_derived.cpp

src/fill_grid.cpp

src/init_geo_grid.cpp

src/inputs.cpp

aburrell

I found one small thing and made a suggestion. You can commit that suggestion directly, or edit it and commit it.

I am going to try and run this branch locally to ensure it runs on more than just your computer before approving. May be asking for help :P

aburrell · 2021-01-11T17:58:17Z

include/constants.h

-const float avogadros_number = 6.02214086e23; //
-const float boltzmanns_constant = 1.38064852e-23; // m2 kg /s2 /K
-const float mass_proton = 1.6726219e-27; // kg
+const float avogadros_number = 6.02214086e23;  //


Suggested change

const float avogadros_number = 6.02214086e23; //

const float avogadros_number = 6.02214086e23; // per mole

aburrell · 2021-01-25T19:23:01Z

Ok, I tried to compile the code and ran into a BUNCH of errors. If the code is not supposed to compile yet, I am ok approving. Otherwise, I can show you the errors.

aaronjridley · 2021-01-25T20:07:38Z

Ok. This is somewhat weird. I branched off of this branch to make the "indices" branch, and noticed a bunch of complication issues also. I wasn't sure if git got confused or whether I didn't commit things (or did commit things and didn't push them). So, I guess I should (a) check out the armadillo branch, (b) see if I can compile the code, (c) if I can/can't, fix the code, and commit changes, and (d) push the changes back to git hub. Right??????

aburrell · 2021-01-25T20:09:47Z

Ok @aaronjridley I have been working on this with @Ionospheres and neither of us can get a working executable on the main branch. I am stuck with a compilation error and @Ionospheres fails in a more sinister fashion. So. I think we should perhaps get together and sort this out before changing more code.

aaronjridley · 2021-01-25T21:05:44Z

Ok, if I clone the repository and then checkout the armadillo branch then try to compile, is there where you all are stuck?
git clone https://github.com/AetherModel/Aether
cd Aether
git checkout armadillo
make
right???

aaronjridley · 2021-01-25T21:08:56Z

When I do this, it definitely doesn't compile. Weird. I must have screwed up somewhere. I will fix, then commit and push. Then let you know when I finish this up. Sorry.

aaronjridley · 2021-01-25T21:57:21Z

Ok, the code should compile and run now. I had found all of these bugs in the "indices" branch. Oops. This must have been a mix-up in the commits and pushes and making new branches and stuff. I probably didn't push these or commit them or something and they got overwritten or something. But, it all works.

aburrell · 2021-01-27T15:20:34Z

I am having issues with netCDF/g++ so @aaronjridley I would recommend asking for a review from someone else. That way we can get this merged in whilst I continue to fix my setup.

Ionospheres · 2021-01-28T19:30:55Z

include/calc_chemistry.h

+// Copyright 2020, the Aether Development Team (see doc/dev_team.md for members)
 // Full license can be found in License.md

 #ifndef AETHER_INCLUDE_CALC_CHEMISTRY_H_


Suggested change

#ifndef AETHER_INCLUDE_CALC_CHEMISTRY_H_

#ifndef INCLUDE_CALC_CHEMISTRY_H_

Ionospheres · 2021-01-28T19:32:35Z

include/calc_chemistry.h

 // Full license can be found in License.md

 #ifndef AETHER_INCLUDE_CALC_CHEMISTRY_H_
 #define AETHER_INCLUDE_CALC_CHEMISTRY_H_


Suggested change

#define AETHER_INCLUDE_CALC_CHEMISTRY_H_

#define INCLUDE_CALC_CHEMISTRY_H_

Ionospheres · 2021-01-28T19:33:14Z

include/calc_chemistry.h

-
-
-#endif // AETHER_INCLUDE_CALC_CHEMISTRY_H_
+#endif  // AETHER_INCLUDE_CALC_CHEMISTRY_H_


Suggested change

#endif // AETHER_INCLUDE_CALC_CHEMISTRY_H_

#endif // INCLUDE_CALC_CHEMISTRY_H_

Ionospheres · 2021-01-28T19:40:17Z

include/grid.h

 For example:
  _s3gc : scalar variable, 3d, include ghost cells, cell centers
-  _31ne : 
+  _31ne :


What would this one be an example of?

Also there are _scgc in use later. This would be: scalar variable, cube?, include ghost cells, cell centers ?

Ionospheres · 2021-01-28T21:48:07Z

include/euv.h

+// Copyright 2020, the Aether Development Team (see doc/dev_team.md for members)
 // Full license can be found in License.md

 #ifndef AETHER_INCLUDE_EUV_H_


Suggested change

#ifndef AETHER_INCLUDE_EUV_H_

#ifndef INCLUDE_EUV_H_

Ionospheres · 2021-01-28T21:48:46Z

include/euv.h

 };

-#endif // AETHER_INCLUDE_EUV_H_
+#endif  // AETHER_INCLUDE_EUV_H_


Suggested change

#endif // AETHER_INCLUDE_EUV_H_

#endif // INCLUDE_EUV_H_

Ionospheres

My first github review! Sorry for all of the update emails. Angeline showed me how to avoid that next time. Overall code looks good. I've made a few format suggestions so things are consistent between files. Outputs look reasonable given the resolutions, included physics, and the early state the model is in. It would be nice if the output netcdf files were formatted such that they work with panoply to facilitate quick viewing of simulation results. I'll learn how to make a feature request (?) and add that to our to-do list.

…o armadillo

aaronjridley added 19 commits December 27, 2020 22:55

FEAT: Store nLons, nLats, nAlts; use armidillo cubes for storage

430bfcf

FEAT: Use amidillo cubes for storage and math.

b62401a

FEAT: Move armidillo include files to /opt/local/include

bd45517

FEAT: Use armidillo cube for storage and math; store nLons, nLats, nA…

48bdbc4

…lts.

FEAT: Store number of ghostcells in grid info.

744656f

FEAT: Change the compile command to -I/opt/local/include

3d32733

FEAT: Apply armidillo cube library to conduction+

de0efe9

FEAT: EUV heating now with armidillo cubes. Preparing for chemistry.

f96accd

BUG: index to iAlt mixup.

f3dbd81

FEAT: Chemistry implemented in amadillo cubes.

5124908

FEAT: Finish armidillo cubes in neutrals and ions.

e65b78c

DOC: Add some comments and clean code a bit.

594a94a

FEAT: Output armadillo cube vars instead of float* vars.

807db01

FEAT: Move more grid stuff to armadillo cubes.

5563039

FEAT: Implement magnetic field in armidillo cubes

e5805a3

DEPREC: Remove rest of the 1d c-native arrays.

4a67379

DEPREC: Removed some 1-d stuff that was commented out.

9e2369e

DEPREC: Remove function for 1d stuff.

e2653ca

FEAT: dAlt default changed to 5 km, just to stretch grid.

3fc52de

aburrell reviewed Jan 6, 2021

View reviewed changes

aaronjridley added 4 commits January 11, 2021 10:44

STY: Delinting of code.

f90e755

STY: delinting code!

8175048

STY: Delinting of the code.

5bd0263

DOC: Added a comment for why there are two versions of FLAGS

f21c56c

aburrell mentioned this pull request Jan 11, 2021

[FEAT] Doxygen comments for code #32

Open

3 tasks

aburrell reviewed Jan 11, 2021

View reviewed changes

FEAT: Chemisty from Richards [2011].

72603d0

aaronjridley added 4 commits January 25, 2021 16:22

BUG: long -> int64_t

0b71c8b

BUG: Solvers were named _new now that is removed.

4accd11

STY: Remove cout statement.

22200ba

BUG: took length of outstring instead of instring. Oops.

335d4ea

aburrell requested a review from Ionospheres January 28, 2021 18:16

Ionospheres reviewed Jan 28, 2021

View reviewed changes

Ionospheres reviewed Feb 1, 2021

View reviewed changes

aaronjridley added 5 commits February 2, 2021 21:48

DOC: add comment (per mole)

320b672

STY: change ifndef statement.

767b787

DOC: added more descriptors of the variable types.

e9a5685

STY: remove aether from ifdef statements.

29d9704

Merge branch 'armadillo' of https://github.com/AetherModel/Aether int…

9054e60

…o armadillo

aaronjridley merged commit 21c7929 into develop Feb 3, 2021

aaronjridley deleted the armadillo branch February 3, 2021 14:11

	const float avogadros_number = 6.02214086e23; //
	const float avogadros_number = 6.02214086e23; // per mole

	#ifndef AETHER_INCLUDE_CALC_CHEMISTRY_H_
	#ifndef INCLUDE_CALC_CHEMISTRY_H_

	#define AETHER_INCLUDE_CALC_CHEMISTRY_H_
	#define INCLUDE_CALC_CHEMISTRY_H_



		#endif // AETHER_INCLUDE_CALC_CHEMISTRY_H_
		#endif // AETHER_INCLUDE_CALC_CHEMISTRY_H_

	#endif // AETHER_INCLUDE_CALC_CHEMISTRY_H_
	#endif // INCLUDE_CALC_CHEMISTRY_H_

Conversation

aaronjridley commented Jan 6, 2021 • edited by aburrell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Test configuration

Checklist:

Uh oh!

aburrell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aburrell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aburrell commented Jan 25, 2021

Uh oh!

aaronjridley commented Jan 25, 2021

Uh oh!

aburrell commented Jan 25, 2021

Uh oh!

aaronjridley commented Jan 25, 2021

Uh oh!

aaronjridley commented Jan 25, 2021

Uh oh!

aaronjridley commented Jan 25, 2021

Uh oh!

aburrell commented Jan 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ionospheres left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aaronjridley commented Jan 6, 2021 •

edited by aburrell

Loading